Re: [PATCH v2] videodev2.h: add helper to validate colorspace

[Hans Verkuil]

On 02/21/2018 09:16 PM, Laurent Pinchart wrote:
> Hi Hans,
> 
> On Tuesday, 20 February 2018 10:37:22 EET Hans Verkuil wrote:
>> On 02/19/2018 11:28 PM, Niklas Söderlund wrote:
>>> Hi Hans,
>>>
>>> Thanks for your feedback.
>>>
>>> [snip]
>>>
 Can you then fix v4l2-compliance to stop testing colorspace
 against 0xff
 ?
>>>
>>> For now I can simply relax this test for subdevs with sources and
>>> sinks.
>>
>> You also need to relax it for video nodes with MC drivers, as the DMA
>> engines don't care about colorspaces.
>
> Yes, they do. Many DMA engines can at least do RGB <-> YUV conversions,
> so they should get the colorspace info from their source and pass it on
> to userspace (after correcting for any conversions done by the DMA
> engine).

 Not in the MC case. Video nodes there only model the DMA engine, and are
 thus not aware of colorspaces. What MC drivers do is check at stream on
 time when validating the pipeline that the colorspace set by userspace
 on the video node corresponds to the colorspace on the source pad of the
 connected subdev, but that's only to ensure that userspace gets a
 coherent view of colorspace across the pipeline, not to program the
 hardware. There could be exceptions, but in the general case, the video
 node implementation of an MC driver will accept any colorspace and only
 validate it at stream on time, similarly to how it does for the frame
 size format instance (and in the frame size case it will usually enforce
 min/max limits when the DMA engine limits the frame size).> 
>>> I'm afraid the issue described above by Laurent is what sparked me to
>>> write this commit to begin with. In my never ending VIN Gen3 patch-set I
>>> currency need to carry a patch [1] to implement a hack to make sure
>>> v4l2-compliance do not fail for the VIN Gen3 MC-centric use-case. This
>>> patch was an attempt to be able to validate the colorspace using the
>>> magic value 0xff.
>>
>> This is NOT a magic value. The test that's done here is to memset the
>> format structure with 0xff, then call the ioctl. Afterwards it checks
>> if there are any remaining 0xff bytes left in the struct since it expects
>> the driver to have overwritten it by something else. That's where the 0xff
>> comes from.
> 
> It's no less or more magic than using 0xdeadbeef or any fixed value :-) I 
> think we all agree that it isn't a value that is meant to be handled 
> specifically by drivers, so it's not magic in that sense.
> 
>>> I don't feel strongly for this patch in particular and I'm happy to drop
>>> it.  But I would like to receive some guidance on how to then properly
>>> be able to handle this problem for the MC-centric VIN driver use-case.
>>> One option is as you suggested to relax the test in v4l-compliance to
>>> not check colorspace, but commit [2] is not enough to resolve the issue
>>> for my MC use-case.
>>>
>>> As Laurent stated above, the use-case is that the video device shall
>>> accept any colorspace set from user-space. This colorspace is then only
>>> used as stream on time to validate the MC pipeline. The VIN driver do
>>> not care about colorspace, but I care about not breaking v4l2-compliance
>>> as I find it's a very useful tool :-)
>>
>> I think part of my confusion here is that there are two places where you
>> deal with colorspaces in a DMA engine: first there is a input pad of the
>> DMA engine entity, secondly there is the v4l2_pix_format for the memory
>> description.
>>
>> The second is set by the driver based on what userspace specified for the
>> input pad, together with any changes due to additional conversions such
>> as quantization range and RGB <-> YUV by the DMA engine.
> 
> No, I'm sorry, for MC-based drivers this isn't correct. The media entity that 
> symbolizes the DMA engine indeed has a sink pad, but it's a video node, not a 
> subdev. It thus has no media bus format configured for its sink pad. The 
> closest pad in the pipeline that has a media bus format is the source pad of 
> the subdev connected to the video node.
> 
> There's no communication within the kernel at G/S_FMT time between the video 
> node and its connected subdev. The only time we look at the pipeline as a 
> whole is when starting the stream to validate that the pipeline is correctly 
> configured. We thus have to implement G/S_FMT on the video node without any 
> knowledge about the connected subdev, and thus accept any colorspace.
> 
>> So any colorspace validation is done for the input pad. The question is
>> what that validation should be. It's never been defined.
> 
> No format is set on the video node's entity sink pad for the reason above, so 
> no validation occurs when setting the colorspace on the sink pad as that 
> never 
> happens.

Is this documented anywhere? Certainly VIDIOC_G/S/TRY_FMT doesn't mention it.

It is certainly a surprise to me.

Regards,

Hans

Re: [PATCH v4 00/16] R-Car DU: Convert LVDS code to bridge driver

[Frank Rowand]

On 02/20/18 15:10, Laurent Pinchart wrote:
> Hello,
> 
> This patch series addresses a design mistake that dates back from the initial
> DU support. Support for the LVDS encoders, which are IP cores separate from
> the DU, was bundled in the DU driver. Worse, both the DU and LVDS were
> described through a single DT node.
> 
> To fix the, patches 01/16 and 02/16 define new DT bindings for the LVDS
> encoders, and deprecate their description inside the DU bindings. To retain
> backward compatibility with existing DT, patches 03/16 to 08/16 then patch the
> device tree at runtime to convert the legacy bindings to the new ones.
> 
> With the DT side addressed, patch 09/16 converts the LVDS support code to a
> separate bridge driver. Patches 11/16 to 16/16 then update all the device tree
> sources to the new DU and LVDS encoders bindings.
> 
> I decided to go for live DT patching in patch 08/16 because implementing
> support for both the legacy and new bindings in the driver would have been
> very intrusive, and prevented further cleanups. This version relies more
> heavily on overlays to avoid touching the internals of the OF core compared to
> v2, even if manual fixes to the device tree are still needed.
> 
> Compared to v3, this series uses the OF changeset API to update properties
> instead of accessing the internals of the property structure. This removes the
> local implementation of functions to look up nodes by path and update
> properties. In order to do this, I pulled in Pantelis' patch series titled
> "[PATCH v2 0/5] of: dynamic: Changesets helpers & fixes" at Rob's request, and
> rebased it while taking two small review comments into account.

Wait a minute!  Why are you putting a patch set to modify core devicetree
in the middle of a driver series.  Please pull it out to a separate series.

I'll try to look at the patches, as they are in this series, sometime
tomorrow.  I have a vague memory of unresolved issues from the last
time they were proposed.

Thanks,

Frank


> 
> Rob, I'd like this series to be merged in v4.17. As the changeset helpers are
> now a dependency, I'd need you to merge them early (ideally on top of
> v4.16-rc1) and provide a stable branch, or get your ack to merge them through
> Dave's tree if they don't conflict with what you have and will queue for
> v4.17.
> 
> This version also drops the small fix to the Porter board device tree that has
> been queued for v4.17 already.
> 
> Compared to v2, the biggest change is in patch 03/16. Following Rob's and
> Frank's reviews it was clear that modifying the unflattened DT structure of
> the overlay before applying it wasn't popular. I have thus decided to use one
> overlay source per SoC to move as much of the DT changes to the overlay as
> possible, and only perform manual modifications (that are still needed as some
> of the information is board-specific) on the system DT after applying the
> overlay. As a result the overlay is parsed and applied without being modified.
> 
> Compared to v1, this series update the r8a7792 and r8a7794 device tree sources
> and incorporate review feedback as described by the changelogs of individual
> patches.
> 
> 
> Laurent Pinchart (11):
>   dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings
>   dt-bindings: display: renesas: Deprecate LVDS support in the DU
> bindings
>   drm: rcar-du: Fix legacy DT to create LVDS encoder nodes
>   drm: rcar-du: Convert LVDS encoder code to bridge driver
>   ARM: dts: r8a7790: Convert to new LVDS DT bindings
>   ARM: dts: r8a7791: Convert to new LVDS DT bindings
>   ARM: dts: r8a7792: Convert to new DU DT bindings
>   ARM: dts: r8a7793: Convert to new LVDS DT bindings
>   ARM: dts: r8a7794: Convert to new DU DT bindings
>   arm64: dts: renesas: r8a7795: Convert to new LVDS DT bindings
>   arm64: dts: renesas: r8a7796: Convert to new LVDS DT bindings
> 
> Pantelis Antoniou (5):
>   of: dynamic: Add __of_node_dupv()
>   of: changesets: Introduce changeset helper methods
>   of: changeset: Add of_changeset_node_move method
>   of: unittest: changeset helpers
>   i2c: demux: Use changeset helpers for clarity
> 
>  .../bindings/display/bridge/renesas,lvds.txt   |  56 +++
>  .../devicetree/bindings/display/renesas,du.txt |  31 +-
>  MAINTAINERS|   1 +
>  arch/arm/boot/dts/r8a7790-lager.dts|  22 +-
>  arch/arm/boot/dts/r8a7790.dtsi |  64 ++-
>  arch/arm/boot/dts/r8a7791-koelsch.dts  |  10 +-
>  arch/arm/boot/dts/r8a7791-porter.dts   |  16 +-
>  arch/arm/boot/dts/r8a7791.dtsi |  36 +-
>  arch/arm/boot/dts/r8a7792.dtsi |   1 -
>  arch/arm/boot/dts/r8a7793-gose.dts |  10 +-
>  arch/arm/boot/dts/r8a7793.dtsi |  37 +-
>  arch/arm/boot/dts/r8a7794.dtsi |   1 -
>  .../boot/dts/renesas/r8a7795-es1-salvator-x.dts|   3 +-
>  

[PATCH v5 0/8] R-Car DU: Convert LVDS code to bridge driver

[Laurent Pinchart]

Hello,

This patch series addresses a design mistake that dates back from the initial
DU support. Support for the LVDS encoders, which are IP cores separate from
the DU, was bundled in the DU driver. Worse, both the DU and LVDS were
described through a single DT node.

To fix the, patches 1/8 and 2/8 define new DT bindings for the LVDS
encoders, and deprecate their description inside the DU bindings. To retain
backward compatibility with existing DT, patches 3/8 to 7/8 then patch the
device tree at runtime to convert the legacy bindings to the new ones.

With the DT side addressed, patch 8/8 converts the LVDS support code to a
separate bridge driver.

I decided to go for live DT patching in patch 7/8 because implementing
support for both the legacy and new bindings in the driver would have been
very intrusive, and prevented further cleanups. This version relies more
heavily on overlays to avoid touching the internals of the OF core compared to
v2, even if manual fixes to the device tree are still needed.

As all the patches but the last one have been acked, I plan to send a pull
request by the end of the week if no new issue is discovered.

Compared to v4, the patch "of: changeset: Add of_changeset_node_move method"
has been dropped as it's not needed. The patches that update the DT sources to
the new DU and LVDS bindings have been dropped as well to avoid spamming the
lists as they depend on the driver changes going in first to avoid
regressions and haven't been changed.

Compared to v3, this series uses the OF changeset API to update properties
instead of accessing the internals of the property structure. This removes the
local implementation of functions to look up nodes by path and update
properties. In order to do this, I pulled in Pantelis' patch series titled
"[PATCH v2 0/5] of: dynamic: Changesets helpers & fixes" at Rob's request, and
rebased it while taking two small review comments into account.

Rob, I'd like this series to be merged in v4.17. As the changeset helpers are
now a dependency, I'd need you to merge them early (ideally on top of
v4.16-rc1) and provide a stable branch, or get your ack to merge them through
Dave's tree if they don't conflict with what you have and will queue for
v4.17.

This version also drops the small fix to the Porter board device tree that has
been queued for v4.17 already.

Compared to v2, the biggest change is in patch 7/8. Following Rob's and
Frank's reviews it was clear that modifying the unflattened DT structure of
the overlay before applying it wasn't popular. I have thus decided to use one
overlay source per SoC to move as much of the DT changes to the overlay as
possible, and only perform manual modifications (that are still needed as some
of the information is board-specific) on the system DT after applying the
overlay. As a result the overlay is parsed and applied without being modified.

Compared to v1, this series update the r8a7792 and r8a7794 device tree sources
and incorporate review feedback as described by the changelogs of individual
patches.

Laurent Pinchart (4):
  dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings
  dt-bindings: display: renesas: Deprecate LVDS support in the DU
bindings
  drm: rcar-du: Fix legacy DT to create LVDS encoder nodes
  drm: rcar-du: Convert LVDS encoder code to bridge driver

Pantelis Antoniou (4):
  of: dynamic: Add __of_node_dupv()
  of: changesets: Introduce changeset helper methods
  of: unittest: changeset helpers
  i2c: demux: Use changeset helpers for clarity

 .../bindings/display/bridge/renesas,lvds.txt   |  56 +++
 .../devicetree/bindings/display/renesas,du.txt |  31 +-
 MAINTAINERS|   1 +
 drivers/gpu/drm/rcar-du/Kconfig|   6 +-
 drivers/gpu/drm/rcar-du/Makefile   |  10 +-
 drivers/gpu/drm/rcar-du/rcar_du_drv.c  |  21 +-
 drivers/gpu/drm/rcar-du/rcar_du_drv.h  |   5 -
 drivers/gpu/drm/rcar-du/rcar_du_encoder.c  | 175 +--
 drivers/gpu/drm/rcar-du/rcar_du_encoder.h  |  12 -
 drivers/gpu/drm/rcar-du/rcar_du_kms.c  |  14 +-
 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c  |  93 
 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h  |  24 -
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c  | 238 --
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h  |  64 ---
 drivers/gpu/drm/rcar-du/rcar_du_of.c   | 307 
 drivers/gpu/drm/rcar-du/rcar_du_of.h   |  20 +
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts|  79 
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts|  53 +++
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts|  53 +++
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts|  53 +++
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts|  53 +++
 drivers/gpu/drm/rcar-du/rcar_lvds.c| 524 +
 drivers/i2c/muxes/i2c-demux-pinctrl.c  |  12 +-
 

[PATCH v5 3/8] of: dynamic: Add __of_node_dupv()

[Laurent Pinchart]

From: Pantelis Antoniou 

Add an __of_node_dupv() private method and make __of_node_dup() use it.
This is required for the subsequent changeset accessors which will
make use of it.

Signed-off-by: Pantelis Antoniou 
[Make __of_node_dupv() static]
Signed-off-by: Laurent Pinchart 
Reviewed-by: Rob Herring 
---
 drivers/of/dynamic.c | 29 +++--
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 7bb33d22b4e2..a2f0c45836f9 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -382,8 +382,9 @@ struct property *__of_prop_dup(const struct property *prop, 
gfp_t allocflags)
 }
 
 /**
- * __of_node_dup() - Duplicate or create an empty device node dynamically.
- * @fmt: Format string (plus vargs) for new full name of the device node
+ * __of_node_dupv() - Duplicate or create an empty device node dynamically.
+ * @fmt: Format string for new full name of the device node
+ * @vargs: va_list containing the arugments for the node full name
  *
  * Create an device tree node, either by duplicating an empty node or by 
allocating
  * an empty one suitable for further modification.  The node data are
@@ -391,17 +392,15 @@ struct property *__of_prop_dup(const struct property 
*prop, gfp_t allocflags)
  * OF_DETACHED bits set. Returns the newly allocated node or NULL on out of
  * memory error.
  */
-struct device_node *__of_node_dup(const struct device_node *np, const char 
*fmt, ...)
+static struct device_node *__of_node_dupv(const struct device_node *np,
+   const char *fmt, va_list vargs)
 {
-   va_list vargs;
struct device_node *node;
 
node = kzalloc(sizeof(*node), GFP_KERNEL);
if (!node)
return NULL;
-   va_start(vargs, fmt);
node->full_name = kvasprintf(GFP_KERNEL, fmt, vargs);
-   va_end(vargs);
if (!node->full_name) {
kfree(node);
return NULL;
@@ -433,6 +432,24 @@ struct device_node *__of_node_dup(const struct device_node 
*np, const char *fmt,
return NULL;
 }
 
+/**
+ * __of_node_dup() - Duplicate or create an empty device node dynamically.
+ * @fmt: Format string (plus vargs) for new full name of the device node
+ *
+ * See: __of_node_dupv()
+ */
+struct device_node *__of_node_dup(const struct device_node *np,
+   const char *fmt, ...)
+{
+   va_list vargs;
+   struct device_node *node;
+
+   va_start(vargs, fmt);
+   node = __of_node_dupv(np, fmt, vargs);
+   va_end(vargs);
+   return node;
+}
+
 static void __of_changeset_entry_destroy(struct of_changeset_entry *ce)
 {
of_node_put(ce->np);
-- 
Regards,

Laurent Pinchart

[PATCH v5 2/8] dt-bindings: display: renesas: Deprecate LVDS support in the DU bindings

[Laurent Pinchart]

The internal LVDS encoders now have their own DT bindings, representing
them as part of the DU is deprecated.

Signed-off-by: Laurent Pinchart 
Reviewed-by: Rob Herring 
---
Changes since v1:

- Remove the LVDS reg range from the example
- Remove the reg-names property
---
 .../devicetree/bindings/display/renesas,du.txt | 31 --
 1 file changed, 11 insertions(+), 20 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt 
b/Documentation/devicetree/bindings/display/renesas,du.txt
index cd48aba3bc8c..e79cf9b0ad38 100644
--- a/Documentation/devicetree/bindings/display/renesas,du.txt
+++ b/Documentation/devicetree/bindings/display/renesas,du.txt
@@ -14,12 +14,7 @@ Required Properties:
 - "renesas,du-r8a7795" for R8A7795 (R-Car H3) compatible DU
 - "renesas,du-r8a7796" for R8A7796 (R-Car M3-W) compatible DU
 
-  - reg: A list of base address and length of each memory resource, one for
-each entry in the reg-names property.
-  - reg-names: Name of the memory resources. The DU requires one memory
-resource for the DU core (named "du") and one memory resource for each
-LVDS encoder (named "lvds.x" with "x" being the LVDS controller numerical
-index).
+  - reg: the memory-mapped I/O registers base address and length
 
   - interrupt-parent: phandle of the parent interrupt controller.
   - interrupts: Interrupt specifiers for the DU interrupts.
@@ -29,14 +24,13 @@ Required Properties:
   - clock-names: Name of the clocks. This property is model-dependent.
 - R8A7779 uses a single functional clock. The clock doesn't need to be
   named.
-- All other DU instances use one functional clock per channel and one
-  clock per LVDS encoder (if available). The functional clocks must be
-  named "du.x" with "x" being the channel numerical index. The LVDS clocks
-  must be named "lvds.x" with "x" being the LVDS encoder numerical index.
-- In addition to the functional and encoder clocks, all DU versions also
-  support externally supplied pixel clocks. Those clocks are optional.
-  When supplied they must be named "dclkin.x" with "x" being the input
-  clock numerical index.
+- All other DU instances use one functional clock per channel The
+  functional clocks must be named "du.x" with "x" being the channel
+  numerical index.
+- In addition to the functional clocks, all DU versions also support
+  externally supplied pixel clocks. Those clocks are optional. When
+  supplied they must be named "dclkin.x" with "x" being the input clock
+  numerical index.
 
   - vsps: A list of phandle and channel index tuples to the VSPs that handle
 the memory interfaces for the DU channels. The phandle identifies the VSP
@@ -69,9 +63,7 @@ Example: R8A7795 (R-Car H3) ES2.0 DU
 
du: display@feb0 {
compatible = "renesas,du-r8a7795";
-   reg = <0 0xfeb0 0 0x8>,
- <0 0xfeb9 0 0x14>;
-   reg-names = "du", "lvds.0";
+   reg = <0 0xfeb0 0 0x8>;
interrupts = ,
 ,
 ,
@@ -79,9 +71,8 @@ Example: R8A7795 (R-Car H3) ES2.0 DU
clocks = < CPG_MOD 724>,
 < CPG_MOD 723>,
 < CPG_MOD 722>,
-< CPG_MOD 721>,
-< CPG_MOD 727>;
-   clock-names = "du.0", "du.1", "du.2", "du.3", "lvds.0";
+< CPG_MOD 721>;
+   clock-names = "du.0", "du.1", "du.2", "du.3";
vsps = < 0>, < 0>, < 0>, < 1>;
 
ports {
-- 
Regards,

Laurent Pinchart

[PATCH v5 6/8] i2c: demux: Use changeset helpers for clarity

[Laurent Pinchart]

From: Pantelis Antoniou 

The changeset helpers are easier to use, use them instead of
using the static property.

Signed-off-by: Pantelis Antoniou 
Acked-by: Wolfram Sang 
["okay" -> "ok"]
Signed-off-by: Laurent Pinchart 
---
 drivers/i2c/muxes/i2c-demux-pinctrl.c | 12 +++-
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-demux-pinctrl.c 
b/drivers/i2c/muxes/i2c-demux-pinctrl.c
index 33ce032cb701..0f0046831492 100644
--- a/drivers/i2c/muxes/i2c-demux-pinctrl.c
+++ b/drivers/i2c/muxes/i2c-demux-pinctrl.c
@@ -220,10 +220,7 @@ static int i2c_demux_pinctrl_probe(struct platform_device 
*pdev)
 
priv = devm_kzalloc(>dev, sizeof(*priv)
   + num_chan * sizeof(struct i2c_demux_pinctrl_chan), 
GFP_KERNEL);
-
-   props = devm_kcalloc(>dev, num_chan, sizeof(*props), GFP_KERNEL);
-
-   if (!priv || !props)
+   if (!priv)
return -ENOMEM;
 
err = of_property_read_string(np, "i2c-bus-name", >bus_name);
@@ -241,12 +238,9 @@ static int i2c_demux_pinctrl_probe(struct platform_device 
*pdev)
}
priv->chan[i].parent_np = adap_np;
 
-   props[i].name = devm_kstrdup(>dev, "status", GFP_KERNEL);
-   props[i].value = devm_kstrdup(>dev, "ok", GFP_KERNEL);
-   props[i].length = 3;
-
of_changeset_init(>chan[i].chgset);
-   of_changeset_update_property(>chan[i].chgset, adap_np, 
[i]);
+   of_changeset_update_property_string(>chan[i].chgset,
+   adap_np, "status", "ok");
}
 
priv->num_chan = num_chan;
-- 
Regards,

Laurent Pinchart

[PATCH v5 4/8] of: changesets: Introduce changeset helper methods

[Laurent Pinchart]

From: Pantelis Antoniou 

Changesets are very powerful, but the lack of a helper API
makes using them cumbersome. Introduce a simple copy based
API that makes things considerably easier.

To wit, adding a property using the raw API.

struct property *prop;
prop = kzalloc(sizeof(*prop)), GFP_KERNEL);
prop->name = kstrdup("compatible");
prop->value = kstrdup("foo,bar");
prop->length = strlen(prop->value) + 1;
of_changeset_add_property(ocs, np, prop);

while using the helper API

of_changeset_add_property_string(ocs, np, "compatible",
"foo,bar");

Signed-off-by: Pantelis Antoniou 
[Fixed memory leak in __of_changeset_add_update_property_copy()]
[Fixed cpu to be32 conversion sparse warnings]
[Move include  to fix compilation when !CONFIG_OF_DYNAMIC]
Signed-off-by: Laurent Pinchart 
Reviewed-by: Rob Herring 
---
 drivers/of/dynamic.c | 222 ++
 include/linux/of.h   | 329 +++
 2 files changed, 551 insertions(+)

diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index a2f0c45836f9..f62083921df2 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -910,3 +910,225 @@ int of_changeset_action(struct of_changeset *ocs, 
unsigned long action,
return 0;
 }
 EXPORT_SYMBOL_GPL(of_changeset_action);
+
+/* changeset helpers */
+
+/**
+ * of_changeset_create_device_node - Create an empty device node
+ *
+ * @ocs:   changeset pointer
+ * @parent:parent device node
+ * @fmt:   format string for the node's full_name
+ * @args:  argument list for the format string
+ *
+ * Create an empty device node, marking it as detached and allocated.
+ *
+ * Returns a device node on success, an error encoded pointer otherwise
+ */
+struct device_node *of_changeset_create_device_nodev(
+   struct of_changeset *ocs, struct device_node *parent,
+   const char *fmt, va_list vargs)
+{
+   struct device_node *node;
+
+   node = __of_node_dupv(NULL, fmt, vargs);
+   if (!node)
+   return ERR_PTR(-ENOMEM);
+
+   node->parent = parent;
+   return node;
+}
+EXPORT_SYMBOL_GPL(of_changeset_create_device_nodev);
+
+/**
+ * of_changeset_create_device_node - Create an empty device node
+ *
+ * @ocs:   changeset pointer
+ * @parent:parent device node
+ * @fmt:   Format string for the node's full_name
+ * ... Arguments
+ *
+ * Create an empty device node, marking it as detached and allocated.
+ *
+ * Returns a device node on success, an error encoded pointer otherwise
+ */
+__printf(3, 4) struct device_node *
+of_changeset_create_device_node(struct of_changeset *ocs,
+   struct device_node *parent, const char *fmt, ...)
+{
+   va_list vargs;
+   struct device_node *node;
+
+   va_start(vargs, fmt);
+   node = of_changeset_create_device_nodev(ocs, parent, fmt, vargs);
+   va_end(vargs);
+   return node;
+}
+EXPORT_SYMBOL_GPL(of_changeset_create_device_node);
+
+/**
+ * __of_changeset_add_property_copy - Create/update a new property copying
+ *name & value
+ *
+ * @ocs:   changeset pointer
+ * @np:device node pointer
+ * @name:  name of the property
+ * @value: pointer to the value data
+ * @length:length of the value in bytes
+ * @update:True on update operation
+ *
+ * Adds/updates a property to the changeset by making copies of the name & 
value
+ * entries. The @update parameter controls whether an add or update takes 
place.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+int __of_changeset_add_update_property_copy(struct of_changeset *ocs,
+   struct device_node *np, const char *name, const void *value,
+   int length, bool update)
+{
+   struct property *prop;
+   int ret = -ENOMEM;
+
+   prop = kzalloc(sizeof(*prop), GFP_KERNEL);
+   if (!prop)
+   return -ENOMEM;
+
+   prop->name = kstrdup(name, GFP_KERNEL);
+   if (!prop->name)
+   goto out_err;
+
+   /*
+* NOTE: There is no check for zero length value.
+* In case of a boolean property, this will allocate a value
+* of zero bytes. We do this to work around the use
+* of of_get_property() calls on boolean values.
+*/
+   prop->value = kmemdup(value, length, GFP_KERNEL);
+   if (!prop->value)
+   goto out_err;
+
+   of_property_set_flag(prop, OF_DYNAMIC);
+
+   prop->length = length;
+
+   if (!update)
+   ret = of_changeset_add_property(ocs, np, prop);
+   else
+   ret = of_changeset_update_property(ocs, np, prop);
+
+   if (!ret)
+   return 0;
+
+out_err:
+   kfree(prop->value);
+   kfree(prop->name);
+ 

[PATCH v5 8/8] drm: rcar-du: Convert LVDS encoder code to bridge driver

[Laurent Pinchart]

The LVDS encoders used to be described in DT as part of the DU. They now
have their own DT node, linked to the DU using the OF graph bindings.
This allows moving internal LVDS encoder support to a separate driver
modelled as a DRM bridge. Backward compatibility is retained as legacy
DT is patched live to move to the new bindings.

Signed-off-by: Laurent Pinchart 
---
Changes since v1:

- Update the SPDX headers to use GPL-2.0 instead of GPL-2.0-only
- Update to the -lvds compatible string format
---
 drivers/gpu/drm/rcar-du/Kconfig   |   4 +-
 drivers/gpu/drm/rcar-du/Makefile  |   3 +-
 drivers/gpu/drm/rcar-du/rcar_du_drv.c |  21 +-
 drivers/gpu/drm/rcar-du/rcar_du_drv.h |   5 -
 drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 175 +-
 drivers/gpu/drm/rcar-du/rcar_du_encoder.h |  12 -
 drivers/gpu/drm/rcar-du/rcar_du_kms.c |  14 +-
 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c |  93 --
 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h |  24 --
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 238 --
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h |  64 
 drivers/gpu/drm/rcar-du/rcar_lvds.c   | 524 ++
 12 files changed, 561 insertions(+), 616 deletions(-)
 delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c
 delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h
 delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
 delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_lvds.c

diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
index 3f83352a7313..edde8d4b87a3 100644
--- a/drivers/gpu/drm/rcar-du/Kconfig
+++ b/drivers/gpu/drm/rcar-du/Kconfig
@@ -19,8 +19,8 @@ config DRM_RCAR_DW_HDMI
  Enable support for R-Car Gen3 internal HDMI encoder.
 
 config DRM_RCAR_LVDS
-   bool "R-Car DU LVDS Encoder Support"
-   depends on DRM_RCAR_DU
+   tristate "R-Car DU LVDS Encoder Support"
+   depends on DRM && DRM_BRIDGE && OF
select DRM_PANEL
select OF_FLATTREE
select OF_OVERLAY
diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar-du/Makefile
index 86b337b4be5d..3e58ed93d5b1 100644
--- a/drivers/gpu/drm/rcar-du/Makefile
+++ b/drivers/gpu/drm/rcar-du/Makefile
@@ -4,10 +4,8 @@ rcar-du-drm-y := rcar_du_crtc.o \
 rcar_du_encoder.o \
 rcar_du_group.o \
 rcar_du_kms.o \
-rcar_du_lvdscon.o \
 rcar_du_plane.o
 
-rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)+= rcar_du_lvdsenc.o
 rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)+= rcar_du_of.o \
   rcar_du_of_lvds_r8a7790.dtb.o \
   rcar_du_of_lvds_r8a7791.dtb.o \
@@ -18,3 +16,4 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)+= rcar_du_vsp.o
 
 obj-$(CONFIG_DRM_RCAR_DU)  += rcar-du-drm.o
 obj-$(CONFIG_DRM_RCAR_DW_HDMI) += rcar_dw_hdmi.o
+obj-$(CONFIG_DRM_RCAR_LVDS)+= rcar_lvds.o
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c 
b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index 6e02c762a557..06a3fbdd728a 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -29,6 +29,7 @@
 
 #include "rcar_du_drv.h"
 #include "rcar_du_kms.h"
+#include "rcar_du_of.h"
 #include "rcar_du_regs.h"
 
 /* 
-
@@ -74,7 +75,6 @@ static const struct rcar_du_device_info rzg1_du_r8a7745_info 
= {
.port = 1,
},
},
-   .num_lvds = 0,
 };
 
 static const struct rcar_du_device_info rcar_du_r8a7779_info = {
@@ -95,14 +95,13 @@ static const struct rcar_du_device_info 
rcar_du_r8a7779_info = {
.port = 1,
},
},
-   .num_lvds = 0,
 };
 
 static const struct rcar_du_device_info rcar_du_r8a7790_info = {
.gen = 2,
.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
-   .quirks = RCAR_DU_QUIRK_ALIGN_128B | RCAR_DU_QUIRK_LVDS_LANES,
+   .quirks = RCAR_DU_QUIRK_ALIGN_128B,
.num_crtcs = 3,
.routes = {
/*
@@ -164,7 +163,6 @@ static const struct rcar_du_device_info 
rcar_du_r8a7792_info = {
.port = 1,
},
},
-   .num_lvds = 0,
 };
 
 static const struct rcar_du_device_info rcar_du_r8a7794_info = {
@@ -186,7 +184,6 @@ static const struct rcar_du_device_info 
rcar_du_r8a7794_info = {
.port = 1,
},
},
-   .num_lvds = 0,
 };
 
 static const struct rcar_du_device_info rcar_du_r8a7795_info = {
@@ -434,7 +431,19 @@ static struct platform_driver rcar_du_platform_driver = {
},
 };
 
-module_platform_driver(rcar_du_platform_driver);
+static int __init rcar_du_init(void)
+{
+   

[PATCH v5 7/8] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Laurent Pinchart]

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.

Patching is disabled and will be enabled along with support for the new
DT bindings in the DU driver.

Signed-off-by: Laurent Pinchart 
---
Changes since v3:

- Use the OF changeset API
- Use of_graph_get_endpoint_by_regs()
- Replace hardcoded constants by sizeof()

Changes since v2:

- Update the SPDX headers to use C-style comments in header files
- Removed the manually created __local_fixups__ node
- Perform manual fixups on live DT instead of overlay

Changes since v1:

- Select OF_FLATTREE
- Compile LVDS DT bindings patch code when DRM_RCAR_LVDS is selected
- Update the SPDX headers to use GPL-2.0 instead of GPL-2.0-only
- Turn __dtb_rcar_du_of_lvds_(begin|end) from u8 to char
- Pass void begin and end pointers to rcar_du_of_get_overlay()
- Use of_get_parent() instead of accessing the parent pointer directly
- Find the LVDS endpoints nodes based on the LVDS node instead of the
  root of the overlay
- Update to the -lvds compatible string format
---
 drivers/gpu/drm/rcar-du/Kconfig|   2 +
 drivers/gpu/drm/rcar-du/Makefile   |   7 +-
 drivers/gpu/drm/rcar-du/rcar_du_of.c   | 307 +
 drivers/gpu/drm/rcar-du/rcar_du_of.h   |  20 ++
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts|  79 ++
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts|  53 
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts|  53 
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts|  53 
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts|  53 
 9 files changed, 626 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of.c
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of.h
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts

diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
index 5d0b4b7119af..3f83352a7313 100644
--- 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
help
  Enable support for the R-Car Display Unit embedded LVDS encoders.
 
diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar-du/Makefile
index 0cf5c11030e8..86b337b4be5d 100644
--- a/drivers/gpu/drm/rcar-du/Makefile
+++ b/drivers/gpu/drm/rcar-du/Makefile
@@ -8,7 +8,12 @@ rcar-du-drm-y := rcar_du_crtc.o \
 rcar_du_plane.o
 
 rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)+= rcar_du_lvdsenc.o
-
+rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)+= rcar_du_of.o \
+  rcar_du_of_lvds_r8a7790.dtb.o \
+  rcar_du_of_lvds_r8a7791.dtb.o \
+  rcar_du_of_lvds_r8a7793.dtb.o \
+  rcar_du_of_lvds_r8a7795.dtb.o \
+  rcar_du_of_lvds_r8a7796.dtb.o
 rcar-du-drm-$(CONFIG_DRM_RCAR_VSP) += rcar_du_vsp.o
 
 obj-$(CONFIG_DRM_RCAR_DU)  += rcar-du-drm.o
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of.c 
b/drivers/gpu/drm/rcar-du/rcar_du_of.c
new file mode 100644
index ..12fae8814309
--- /dev/null
+++ b/drivers/gpu/drm/rcar-du/rcar_du_of.c
@@ -0,0 +1,307 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * rcar_du_of.c - Legacy DT bindings compatibility
+ *
+ * Copyright (C) 2018 Laurent Pinchart 
+ *
+ * Based on work from Jyri Sarha 
+ * Copyright (C) 2015 Texas Instruments
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "rcar_du_crtc.h"
+#include "rcar_du_drv.h"
+
+/* 
-
+ * Generic Overlay Handling
+ */
+
+struct rcar_du_of_overlay {
+   const char *compatible;
+   void *begin;
+   void *end;
+};
+
+#define RCAR_DU_OF_DTB(type, soc)  \
+   extern char __dtb_rcar_du_of_##type##_##soc##_begin[];  \
+   extern char __dtb_rcar_du_of_##type##_##soc##_end[]
+
+#define RCAR_DU_OF_OVERLAY(type, soc)  \
+   {   \
+   .compatible = "renesas,du-" #soc,   \
+   .begin = 

[PATCH v5 5/8] of: unittest: changeset helpers

[Laurent Pinchart]

From: Pantelis Antoniou 

Add a unitest specific for the new changeset helpers.

Signed-off-by: Pantelis Antoniou 
[Use IS_ENABLED instead of #ifdef]
Signed-off-by: Laurent Pinchart 
Reviewed-by: Rob Herring 
---
 drivers/of/unittest.c | 54 +++
 1 file changed, 54 insertions(+)

diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 7a9abaae874d..1b21d2c549a8 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -609,6 +609,59 @@ static void __init of_unittest_changeset(void)
 #endif
 }
 
+static void __init of_unittest_changeset_helper(void)
+{
+#ifdef CONFIG_OF_DYNAMIC
+   struct device_node *n1, *n2, *n21, *parent, *np;
+   struct of_changeset chgset;
+
+   of_changeset_init();
+
+   parent = of_find_node_by_path("/testcase-data/changeset");
+
+   unittest(parent, "testcase setup failure\n");
+   n1 = of_changeset_create_device_node(,
+   parent, "/testcase-data/changeset/n1");
+   unittest(n1, "testcase setup failure\n");
+   n2 = of_changeset_create_device_node(,
+   parent, "/testcase-data/changeset/n2");
+   unittest(n2, "testcase setup failure\n");
+   n21 = of_changeset_create_device_node(, n2, "%s/%s",
+   "/testcase-data/changeset/n2", "n21");
+   unittest(n21, "testcase setup failure\n");
+
+   unittest(!of_changeset_add_property_string(, parent,
+   "prop-add", "foo"), "fail add prop\n");
+
+   unittest(!of_changeset_attach_node(, n1), "fail n1 attach\n");
+   unittest(!of_changeset_attach_node(, n2), "fail n2 attach\n");
+   unittest(!of_changeset_attach_node(, n21), "fail n21 attach\n");
+
+   unittest(!of_changeset_apply(), "apply failed\n");
+
+   /* Make sure node names are constructed correctly */
+   np = of_find_node_by_path("/testcase-data/changeset/n1");
+   unittest(np, "'%s' not added\n", n1->full_name);
+   of_node_put(np);
+
+   /* Make sure node names are constructed correctly */
+   np = of_find_node_by_path("/testcase-data/changeset/n2");
+   unittest(np, "'%s' not added\n", n2->full_name);
+   of_node_put(np);
+
+   np = of_find_node_by_path("/testcase-data/changeset/n2/n21");
+   unittest(np, "'%s' not added\n", n21->full_name);
+   of_node_put(np);
+
+   unittest(!of_changeset_revert(), "revert failed\n");
+
+   of_changeset_destroy();
+
+   of_node_put(parent);
+#endif
+}
+
+
 static void __init of_unittest_parse_interrupts(void)
 {
struct device_node *np;
@@ -2363,6 +2416,7 @@ static int __init of_unittest(void)
of_unittest_property_string();
of_unittest_property_copy();
of_unittest_changeset();
+   of_unittest_changeset_helper();
of_unittest_parse_interrupts();
of_unittest_parse_interrupts_extended();
of_unittest_match_node();
-- 
Regards,

Laurent Pinchart

[PATCH v5 1/8] dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings

[Laurent Pinchart]

The Renesas R-Car Gen2 and Gen3 SoCs have internal LVDS encoders. Add
corresponding device tree bindings.

Signed-off-by: Laurent Pinchart 
Reviewed-by: Rob Herring 
---
Changes since v1:

- Move the SoC name before the IP name in compatible strings
- Rename parallel input to parallel RGB input
- Fixed "renesas,r8a7743-lvds" description
- Document the resets property
- Fixed typo
---
 .../bindings/display/bridge/renesas,lvds.txt   | 56 ++
 MAINTAINERS|  1 +
 2 files changed, 57 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt

diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt 
b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
new file mode 100644
index ..2b19ce51ec07
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
@@ -0,0 +1,56 @@
+Renesas R-Car LVDS Encoder
+==
+
+These DT bindings describe the LVDS encoder embedded in the Renesas R-Car
+Gen2, R-Car Gen3 and RZ/G SoCs.
+
+Required properties:
+
+- compatible : Shall contain one of
+  - "renesas,r8a7743-lvds" for R8A7743 (RZ/G1M) compatible LVDS encoders
+  - "renesas,r8a7790-lvds" for R8A7790 (R-Car H2) compatible LVDS encoders
+  - "renesas,r8a7791-lvds" for R8A7791 (R-Car M2-W) compatible LVDS encoders
+  - "renesas,r8a7793-lvds" for R8A7791 (R-Car M2-N) compatible LVDS encoders
+  - "renesas,r8a7795-lvds" for R8A7795 (R-Car H3) compatible LVDS encoders
+  - "renesas,r8a7796-lvds" for R8A7796 (R-Car M3-W) compatible LVDS encoders
+
+- reg: Base address and length for the memory-mapped registers
+- clocks: A phandle + clock-specifier pair for the functional clock
+- resets: A phandle + reset specifier for the module reset
+
+Required nodes:
+
+The LVDS encoder has two video ports. Their connections are modelled using the
+OF graph bindings specified in Documentation/devicetree/bindings/graph.txt.
+
+- Video port 0 corresponds to the parallel RGB input
+- Video port 1 corresponds to the LVDS output
+
+Each port shall have a single endpoint.
+
+
+Example:
+
+   lvds0: lvds@feb9 {
+   compatible = "renesas,r8a7790-lvds";
+   reg = <0 0xfeb9 0 0x1c>;
+   clocks = < CPG_MOD 726>;
+   resets = < 726>;
+
+   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 {
+   };
+   };
+   };
+   };
diff --git a/MAINTAINERS b/MAINTAINERS
index 2afba909724c..13c8ec11135a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4744,6 +4744,7 @@ F:drivers/gpu/drm/rcar-du/
 F: drivers/gpu/drm/shmobile/
 F: include/linux/platform_data/shmob_drm.h
 F: Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt
+F: Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
 F: Documentation/devicetree/bindings/display/renesas,du.txt
 
 DRM DRIVERS FOR ROCKCHIP
-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 08/16] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Laurent Pinchart]

Hi Rob,

On Thursday, 22 February 2018 01:28:48 EET Rob Herring wrote:
> On Tue, Feb 20, 2018 at 5:10 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.
> > 
> > Patching is disabled and will be enabled along with support for the new
> > DT bindings in the DU driver.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> > ---
> 
> [...]
> 
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts
> > b/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts new file mode
> > 100644
> > index ..6ebb355b652a
> > --- /dev/null
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts
> > @@ -0,0 +1,81 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * rcar_du_of_lvds_r8a7790.dts - Legacy LVDS DT bindings conversion for
> > R8A7790
> > + *
> > + * Copyright (C) 2018 Laurent Pinchart
> > 
> > + *
> > + * Based on work from Jyri Sarha 
> > + * Copyright (C) 2015 Texas Instruments
> > + */
> > +
> > +#include 
> 
> Doesn't seem to be used in any of these.

It's a leftover from a previous test. I'll remove it in all the .dts files.

> Otherwise,
> 
> Reviewed-by: Rob Herring 
> 
> > +
> > +/dts-v1/;
> > +/plugin/;
> > +/ {
> > +   fragment@0 {
> > +   target-path = "/";
> > +   __overlay__ {
> > +   #address-cells = <2>;
> > +   #size-cells = <2>;
> > +
> > +   lvds@feb9 {
> > +   compatible = "renesas,r8a7790-lvds";
> > +   reg = <0 0xfeb9 0 0x1c>;
> > +
> > +   ports {
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +
> > +   port@0 {
> > +   reg = <0>;
> > +   lvds0_input: endpoint {
> > +   };
> > +   };
> > +   port@1 {
> > +   reg = <1>;
> > +   lvds0_out: endpoint {
> > +   };
> > +   };
> > +   };
> > +   };
> > +
> > +   lvds@feb94000 {
> > +   compatible = "renesas,r8a7790-lvds";
> > +   reg = <0 0xfeb94000 0 0x1c>;
> > +
> > +   ports {
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +
> > +   port@0 {
> > +   reg = <0>;
> > +   lvds1_input: endpoint {
> > +   };
> > +   };
> > +   port@1 {
> > +   reg = <1>;
> > +   lvds1_out: endpoint {
> > +   };
> > +   };
> > +   };
> > +   };
> > +   };
> > +   };
> > +
> > +   fragment@1 {
> > +   target-path = "/display@feb0/ports";
> > +   __overlay__ {
> > +   port@1 {
> > +   endpoint {
> > +   remote-endpoint = <_input>;
> > +   };
> > +   };
> > +   port@2 {
> > +   endpoint {
> > +   remote-endpoint = <_input>;
> > +   };
> > +   };
> > +   };
> > +   };
> > +};

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 06/16] of: unittest: changeset helpers

[Rob Herring]

On Wed, Feb 21, 2018 at 5:39 PM, Laurent Pinchart
 wrote:
> Hi Rob,
>
> On Thursday, 22 February 2018 01:10:25 EET Rob Herring wrote:
>> On Tue, Feb 20, 2018 at 5:10 PM, Laurent Pinchart wrote:
>> > From: Pantelis Antoniou 
>> >
>> > Add a unitest specific for the new changeset helpers.
>> >
>> > Signed-off-by: Pantelis Antoniou 
>> > Signed-off-by: Laurent Pinchart
>> > 
>> > ---
>> >
>> >  drivers/of/unittest.c | 54 ++
>> >  1 file changed, 54 insertions(+)
>> >
>> > diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
>> > index 7a9abaae874d..1b21d2c549a8 100644
>> > --- a/drivers/of/unittest.c
>> > +++ b/drivers/of/unittest.c
>> > @@ -609,6 +609,59 @@ static void __init of_unittest_changeset(void)
>> >
>> >  #endif
>> >  }
>> >
>> > +static void __init of_unittest_changeset_helper(void)
>> > +{
>> > +#ifdef CONFIG_OF_DYNAMIC
>>
>> I think this can be:
>>
>> if (!IS_ENABLED(CONFIG_OF_DYNAMIC))
>>   return;
>
> Not quite, as there are functions used below (such as of_changeset_init())
> that are not defined if CONFIG_OF_DYNAMIC isn't enabled. We could create stubs
> in that case but I believe that's out of scope for this patch series.

Okay. I thought we had the necessary stubs, but didn't check too closely.

>
>> Otherwise,
>>
>> Reviewed-by: Rob Herring 
>
> --
> Regards,
>
> Laurent Pinchart
>

Re: [PATCH v4 05/16] of: changeset: Add of_changeset_node_move method

[Laurent Pinchart]

Hi Rob,

On Thursday, 22 February 2018 01:20:36 EET Rob Herring wrote:
> On Tue, Feb 20, 2018 at 5:10 PM, Laurent Pinchart wrote:
> > From: Pantelis Antoniou 
> > 
> > Adds a changeset helper for moving a subtree to a different place
> > in the running tree. This is useful in advances cases of dynamic
> > device tree construction.
> 
> This one I'm not real clear on when we'd use this and we don't have
> any user, so lets drop it for now.

OK, no problem.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 06/16] of: unittest: changeset helpers

[Laurent Pinchart]

Hi Rob,

On Thursday, 22 February 2018 01:10:25 EET Rob Herring wrote:
> On Tue, Feb 20, 2018 at 5:10 PM, Laurent Pinchart wrote:
> > From: Pantelis Antoniou 
> > 
> > Add a unitest specific for the new changeset helpers.
> > 
> > Signed-off-by: Pantelis Antoniou 
> > Signed-off-by: Laurent Pinchart
> > 
> > ---
> > 
> >  drivers/of/unittest.c | 54 ++
> >  1 file changed, 54 insertions(+)
> > 
> > diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> > index 7a9abaae874d..1b21d2c549a8 100644
> > --- a/drivers/of/unittest.c
> > +++ b/drivers/of/unittest.c
> > @@ -609,6 +609,59 @@ static void __init of_unittest_changeset(void)
> > 
> >  #endif
> >  }
> > 
> > +static void __init of_unittest_changeset_helper(void)
> > +{
> > +#ifdef CONFIG_OF_DYNAMIC
> 
> I think this can be:
> 
> if (!IS_ENABLED(CONFIG_OF_DYNAMIC))
>   return;

Not quite, as there are functions used below (such as of_changeset_init()) 
that are not defined if CONFIG_OF_DYNAMIC isn't enabled. We could create stubs 
in that case but I believe that's out of scope for this patch series.

> Otherwise,
> 
> Reviewed-by: Rob Herring 

-- 
Regards,

Laurent Pinchart

[PATCH] pinctrl: sh-pfc: r8a7795: remove duplicate of CLKOUT pin in pinmux_pins[]

[Niklas Söderlund]

When adding GP-1-28 port pin support it was forgotten to remove the
CLKOUT pin from the list of pins that are not associated with a GPIO
port in pinmux_pins[]. This results in a warning when reading the
pinctrl files in sysfs as the CLKOUT pin is still added as a none GPIO
pin. Fix this by removing the duplicated entry which is no longer
needed.

~ # cat /sys/kernel/debug/pinctrl/e606.pin-controller/pinconf-pins
[   89.432081] [ cut here ]
[   89.436904] Pin 496 is not in bias info list
[   89.441252] WARNING: CPU: 1 PID: 456 at drivers/pinctrl/sh-pfc/core.c:408 
sh_pfc_pin_to_bias_reg+0xb0/0xb8
[   89.451002] CPU: 1 PID: 456 Comm: cat Not tainted 
4.16.0-rc1-arm64-renesas-00048-gdfafc344a4f24dde #12
[   89.460394] Hardware name: Renesas Salvator-X 2nd version board based on 
r8a7795 ES2.0+ (DT)
[   89.468910] pstate: 8085 (Nzcv daIf -PAN -UAO)
[   89.473747] pc : sh_pfc_pin_to_bias_reg+0xb0/0xb8
[   89.478495] lr : sh_pfc_pin_to_bias_reg+0xb0/0xb8
[   89.483241] sp : 0aff3ab0
[   89.486587] x29: 0aff3ab0 x28: 0893c698
[   89.491955] x27: 08ad7d98 x26: 
[   89.497323] x25: 8006fb3f5028 x24: 8006fb3f5018
[   89.502690] x23: 0001 x22: 01f0
[   89.508057] x21: 8006fb3f5018 x20: 08bef000
[   89.513423] x19:  x18: 
[   89.518790] x17: 6c4a x16: 08d67c98
[   89.524157] x15: 0001 x14: 0896ca98
[   89.529524] x13: cce5f611 x12: 8006f8d3b5a8
[   89.534891] x11: 0981e000 x10: 08befa08
[   89.540258] x9 : 8006f9b987a0 x8 : 08befa08
[   89.545625] x7 : 08137094 x6 : 
[   89.550991] x5 :  x4 : 0001
[   89.556357] x3 : 0007 x2 : 0007
[   89.561723] x1 : 1ff24f80f1818600 x0 : 
[   89.567091] Call trace:
[   89.569561]  sh_pfc_pin_to_bias_reg+0xb0/0xb8
[   89.573960]  r8a7795_pinmux_get_bias+0x30/0xc0
[   89.578445]  sh_pfc_pinconf_get+0x1e0/0x2d8
[   89.582669]  pin_config_get_for_pin+0x20/0x30
[   89.587067]  pinconf_generic_dump_one+0x180/0x1c8
[   89.591815]  pinconf_generic_dump_pins+0x84/0xd8
[   89.596476]  pinconf_pins_show+0xc8/0x130
[   89.600528]  seq_read+0xe4/0x510
[   89.603789]  full_proxy_read+0x60/0x90
[   89.607576]  __vfs_read+0x30/0x140
[   89.611010]  vfs_read+0x90/0x170
[   89.614269]  SyS_read+0x60/0xd8
[   89.617443]  __sys_trace_return+0x0/0x4
[   89.621314] ---[ end trace 99c8d0d39c13e794 ]---

Fixes: 82d2de5a4f646f72 ("pinctrl: sh-pfc: r8a7795: Add GP-1-28 port pin 
support")
Signed-off-by: Niklas Söderlund 
---
 drivers/pinctrl/sh-pfc/pfc-r8a7795.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c 
b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
index 18aeee592fdcf246..35951e7b89d2fae3 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
@@ -1538,7 +1538,6 @@ static const struct sh_pfc_pin pinmux_pins[] = {
SH_PFC_PIN_NAMED_CFG('B', 18, AVB_TD1, CFG_FLAGS),
SH_PFC_PIN_NAMED_CFG('B', 19, AVB_RXC, CFG_FLAGS),
SH_PFC_PIN_NAMED_CFG('C',  1, PRESETOUT#, CFG_FLAGS),
-   SH_PFC_PIN_NAMED_CFG('F',  1, CLKOUT, CFG_FLAGS),
SH_PFC_PIN_NAMED_CFG('H', 37, MLB_REF, CFG_FLAGS),
SH_PFC_PIN_NAMED_CFG('V',  3, QSPI1_SPCLK, CFG_FLAGS),
SH_PFC_PIN_NAMED_CFG('V',  5, QSPI1_SSL, CFG_FLAGS),
-- 
2.16.1

Re: [PATCH v4 08/16] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Rob Herring]

On Tue, Feb 20, 2018 at 5:10 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.
>
> Patching is disabled and will be enabled along with support for the new
> DT bindings in the DU driver.
>
> Signed-off-by: Laurent Pinchart 
> ---

[...]

> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts 
> b/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts
> new file mode 100644
> index ..6ebb355b652a
> --- /dev/null
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts
> @@ -0,0 +1,81 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * rcar_du_of_lvds_r8a7790.dts - Legacy LVDS DT bindings conversion for 
> R8A7790
> + *
> + * Copyright (C) 2018 Laurent Pinchart 
> + *
> + * Based on work from Jyri Sarha 
> + * Copyright (C) 2015 Texas Instruments
> + */
> +
> +#include 

Doesn't seem to be used in any of these.

Otherwise,

Reviewed-by: Rob Herring 

> +
> +/dts-v1/;
> +/plugin/;
> +/ {
> +   fragment@0 {
> +   target-path = "/";
> +   __overlay__ {
> +   #address-cells = <2>;
> +   #size-cells = <2>;
> +
> +   lvds@feb9 {
> +   compatible = "renesas,r8a7790-lvds";
> +   reg = <0 0xfeb9 0 0x1c>;
> +
> +   ports {
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +
> +   port@0 {
> +   reg = <0>;
> +   lvds0_input: endpoint {
> +   };
> +   };
> +   port@1 {
> +   reg = <1>;
> +   lvds0_out: endpoint {
> +   };
> +   };
> +   };
> +   };
> +
> +   lvds@feb94000 {
> +   compatible = "renesas,r8a7790-lvds";
> +   reg = <0 0xfeb94000 0 0x1c>;
> +
> +   ports {
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +
> +   port@0 {
> +   reg = <0>;
> +   lvds1_input: endpoint {
> +   };
> +   };
> +   port@1 {
> +   reg = <1>;
> +   lvds1_out: endpoint {
> +   };
> +   };
> +   };
> +   };
> +   };
> +   };
> +
> +   fragment@1 {
> +   target-path = "/display@feb0/ports";
> +   __overlay__ {
> +   port@1 {
> +   endpoint {
> +   remote-endpoint = <_input>;
> +   };
> +   };
> +   port@2 {
> +   endpoint {
> +   remote-endpoint = <_input>;
> +   };
> +   };
> +   };
> +   };
> +};

Re: [PATCH v4 05/16] of: changeset: Add of_changeset_node_move method

[Rob Herring]

On Tue, Feb 20, 2018 at 5:10 PM, Laurent Pinchart
 wrote:
> From: Pantelis Antoniou 
>
> Adds a changeset helper for moving a subtree to a different place
> in the running tree. This is useful in advances cases of dynamic
> device tree construction.

This one I'm not real clear on when we'd use this and we don't have
any user, so lets drop it for now.

Rob

[PATCH] i2c: adv748x: afe: fix sparse warning

[Niklas Söderlund]

This fixes the following sparse warning:

drivers/media/i2c/adv748x/adv748x-afe.c:294:34:expected unsigned int 
[usertype] *signal
drivers/media/i2c/adv748x/adv748x-afe.c:294:34:got int *
drivers/media/i2c/adv748x/adv748x-afe.c:294:34: warning: incorrect type in 
argument 2 (different signedness)

Signed-off-by: Niklas Söderlund 
---
 drivers/media/i2c/adv748x/adv748x-afe.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/adv748x/adv748x-afe.c 
b/drivers/media/i2c/adv748x/adv748x-afe.c
index 5188178588c9067d..39a9996d0db08c31 100644
--- a/drivers/media/i2c/adv748x/adv748x-afe.c
+++ b/drivers/media/i2c/adv748x/adv748x-afe.c
@@ -275,7 +275,8 @@ static int adv748x_afe_s_stream(struct v4l2_subdev *sd, int 
enable)
 {
struct adv748x_afe *afe = adv748x_sd_to_afe(sd);
struct adv748x_state *state = adv748x_afe_to_state(afe);
-   int ret, signal = V4L2_IN_ST_NO_SIGNAL;
+   unsigned int signal = V4L2_IN_ST_NO_SIGNAL;
+   int ret;
 
mutex_lock(>mutex);
 
-- 
2.16.1

Re: [PATCH v4 03/16] of: dynamic: Add __of_node_dupv()

[Rob Herring]

On Tue, Feb 20, 2018 at 5:10 PM, Laurent Pinchart
 wrote:
> From: Pantelis Antoniou 
>
> Add an __of_node_dupv() private method and make __of_node_dup() use it.
> This is required for the subsequent changeset accessors which will
> make use of it.
>
> Signed-off-by: Pantelis Antoniou 
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/of/dynamic.c | 29 +++--
>  1 file changed, 23 insertions(+), 6 deletions(-)

Reviewed-by: Rob Herring 

Re: [PATCH v4 04/16] of: changesets: Introduce changeset helper methods

[Rob Herring]

On Tue, Feb 20, 2018 at 5:10 PM, Laurent Pinchart
 wrote:
> From: Pantelis Antoniou 
>
> Changesets are very powerful, but the lack of a helper API
> makes using them cumbersome. Introduce a simple copy based
> API that makes things considerably easier.
>
> To wit, adding a property using the raw API.
>
> struct property *prop;
> prop = kzalloc(sizeof(*prop)), GFP_KERNEL);
> prop->name = kstrdup("compatible");
> prop->value = kstrdup("foo,bar");
> prop->length = strlen(prop->value) + 1;
> of_changeset_add_property(ocs, np, prop);
>
> while using the helper API
>
> of_changeset_add_property_string(ocs, np, "compatible",
> "foo,bar");
>
> Signed-off-by: Pantelis Antoniou 
> [Fixed memory leak in __of_changeset_add_update_property_copy()]
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/of/dynamic.c | 222 ++
>  include/linux/of.h   | 328 
> +++
>  2 files changed, 550 insertions(+)

Other than what Geert pointed out,

Reviewed-by: Rob Herring 

Re: [PATCH v4 06/16] of: unittest: changeset helpers

[Rob Herring]

On Tue, Feb 20, 2018 at 5:10 PM, Laurent Pinchart
 wrote:
> From: Pantelis Antoniou 
>
> Add a unitest specific for the new changeset helpers.
>
> Signed-off-by: Pantelis Antoniou 
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/of/unittest.c | 54 
> +++
>  1 file changed, 54 insertions(+)
>
> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> index 7a9abaae874d..1b21d2c549a8 100644
> --- a/drivers/of/unittest.c
> +++ b/drivers/of/unittest.c
> @@ -609,6 +609,59 @@ static void __init of_unittest_changeset(void)
>  #endif
>  }
>
> +static void __init of_unittest_changeset_helper(void)
> +{
> +#ifdef CONFIG_OF_DYNAMIC

I think this can be:

if (!IS_ENABLED(CONFIG_OF_DYNAMIC))
  return;

Otherwise,

Reviewed-by: Rob Herring 

Re: [PATCH v10 04/10] ARM: dts: r7s72100: Add Capture Engine Unit (CEU)

[Simon Horman]

On Wed, Feb 21, 2018 at 06:47:58PM +0100, 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 

This patch depends on the binding for "renesas,r7s72100-ceu".
Please repost or otherwise ping me once that dependency has been accepted.

Re: [PATCH 1/2] ARM: shmobile: Factor out complex condition

[Marek Vasut]

On 02/19/2018 09:58 AM, Simon Horman wrote:
> On Sat, Feb 17, 2018 at 03:06:41AM +0100, Marek Vasut wrote:
>> Pull the complex condition in regulator_quirk_notify() into
>> regulator_quirk_check(). Moreover, do not hard-code the I2C
>> address, but rather use the one in da9xxx_msgs[].
>>
>> Signed-off-by: Marek Vasut 
>> Cc: Geert Uytterhoeven 
>> Cc: Kuninori Morimoto 
>> Cc: Simon Horman 
>> Cc: Wolfram Sang 
> 
> There appears to be some review of this series that needs addressing,
> I have marked it as "Changes Requested".
> 
Jupp

-- 
Best regards,
Marek Vasut

Re: [PATCH v2] videodev2.h: add helper to validate colorspace

[Sakari Ailus]

Hi Laurent and Hans,

On Wed, Feb 21, 2018 at 10:16:25PM +0200, Laurent Pinchart wrote:
> No, I'm sorry, for MC-based drivers this isn't correct. The media entity that 
> symbolizes the DMA engine indeed has a sink pad, but it's a video node, not a 
> subdev. It thus has no media bus format configured for its sink pad. The 
> closest pad in the pipeline that has a media bus format is the source pad of 
> the subdev connected to the video node.
> 
> There's no communication within the kernel at G/S_FMT time between the video 
> node and its connected subdev. The only time we look at the pipeline as a 
> whole is when starting the stream to validate that the pipeline is correctly 
> configured. We thus have to implement G/S_FMT on the video node without any 
> knowledge about the connected subdev, and thus accept any colorspace.

A few more notes related to this --- there's no propagation of sub-device
format across the entities; there were several reasons for the design
choice. The V4L2 pixel format in the video node must be compatible with the
sub-device format when streaming is started. And... the streaming will
start in the pipeline defined by the enabled links to/from the video node.
In principle nothign prevents having multiple links there, and at the time
S_FMT IOCTL is called on the video node, none of those links could be
enabled. And that's perfectly valid use of the API.

A lot of the DMA engine drivers are simply devices that receive data and
write that to system memory, they really don't necessarily know anything
else. For the hardware this data is just pixels (or even bits, especially
in the case of CSI-2!).

So I agree with Laurent here that requiring correct colour space for
[GS]_FMT IOCTLs on video nodes in the general case is not feasible
(especially on MC-centric devices), due to the way the Media controller
pipeline and formats along that pipeline are configured and validated (i.e.
at streamon time).

But what to do here then? The colourspace field is not verified even in
link validation so there's no guarantee it's correctly set more or less
anywhere else than in the source of the stream. And if the stream has
multiple sources with different colour spaces, then what do you do? That's
perhaps a theoretical case but the current frameworks and APIs do in
principle support that.

Perhaps we should specify that the user should always set the same
colourspace on the sink end of a link that was there in the source? The
same should likely apply to the rest of the fields apart from width, height
and code, too. Before the user configures formats this doesn't work though,
and this does not address the matter with v4l2-compliance.

Granted that the drivers will themselves handle the colour space
information correctly, it'd still provide a way for the user to gain the
knowledge of the colour space which I believe is what matters.

-- 
Kind regards,

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

Re: [PATCH] clk: renesas: rcar-gen3: Fix SD divider setting

[Wolfram Sang]

Simon,

sorry for the delay in checking this. I refreshed my memory about the
issues here this evening and will be able to do educated tests tomorrow.

However, one question already:

> - In M3N, HS400 mode and HS200 mode use the same clock setting.

a) I did not find any code handling this speciality for M3N, neither
   here nor in the SDHI driver. Did you notice something in the BSP?

b) do you know the reasons for this? Looking at the datasheet I have,
   sdsrc is 800MHz for all Gen3 SoC.

> On H3 ES1.0 / Salvator-X I do not see any problems either with or without
> this patch.

I assume this is because the speed is 200MHz instead of 400MHz. The
sdsrc clk is half speed on Salvator-X-H3-ES1.0. I hope I can confirm my
assumption tomorrow.

> https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git 
> topic/hs400-v2

Thanks, that was useful!

Regards,

   Wolfram



signature.asc
Description: PGP signature

Re: [PATCH v9 11/11] media: i2c: ov7670: Fully set mbus frame fmt

[Laurent Pinchart]

Hi Jacopo,

On Tuesday, 20 February 2018 10:58:57 EET jacopo mondi wrote:
> On Mon, Feb 19, 2018 at 09:19:32PM +0200, Laurent Pinchart wrote:
> > On Monday, 19 February 2018 18:59:44 EET Jacopo Mondi wrote:
> >> The sensor driver sets mbus format colorspace information and sizes,
> >> but not ycbcr encoding, quantization and xfer function. When supplied
> >> with an badly initialized mbus frame format structure, those fields
> >> need to be set explicitly not to leave them uninitialized. This is
> >> tested by v4l2-compliance, which supplies a mbus format description
> >> structure and checks for all fields to be properly set.
> >> 
> >> Without this commit, v4l2-compliance fails when testing formats with:
> >> fail: v4l2-test-formats.cpp(335): ycbcr_enc >= 0xff
> >> 
> >> Signed-off-by: Jacopo Mondi 
> >> ---
> >> 
> >>  drivers/media/i2c/ov7670.c | 4 
> >>  1 file changed, 4 insertions(+)
> >> 
> >> diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
> >> index 25b26d4..61c472e 100644
> >> --- a/drivers/media/i2c/ov7670.c
> >> +++ b/drivers/media/i2c/ov7670.c
> >> @@ -996,6 +996,10 @@ static int ov7670_try_fmt_internal(struct
> >> v4l2_subdev
> >> *sd, fmt->height = wsize->height;
> >> 
> >>fmt->colorspace = ov7670_formats[index].colorspace;
> > 
> > On a side note, if I'm not mistaken the colorspace field is set to SRGB
> > for all entries. Shouldn't you hardcode it here and remove the field ?
> > 
> >> +  fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> >> +  fmt->quantization = V4L2_QUANTIZATION_DEFAULT;
> >> +  fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT;
> > 
> > How about setting the values explicitly instead of relying on defaults ?
> > That would be V4L2_YCBCR_ENC_601, V4L2_QUANTIZATION_LIM_RANGE and
> > V4L2_XFER_FUNC_SRGB. And could you then check a captured frame to see if
> > the sensor outputs limited or full range ?
> 
> This actually makes me wonder if those informations (ycbcb_enc,
> quantization and xfer_func) shouldn't actually be part of the
> supported format list... I blindly added those default fields in the
> try_fmt function, but I doubt they applies to all supported formats.
> 
> Eg. the sensor supports YUYV as well as 2 RGB encodings (RGB444 and
> RGB 565) and 1 raw format (BGGR). I now have a question here:
> 
> 1) ycbcr_enc transforms non-linear R'G'B' to Y'CbCr: does this
> applies to RGB and raw formats? I don't think so, and what value is
> the correct one for the ycbcr_enc field in this case? I assume
> xfer_func and quantization applies to all formats instead..

There's no encoding for RGB formats if I understand things correctly, so I'd 
set ycbcr_enc to V4L2_YCBCR_ENC_DEFAULT. The transfer function and the 
quantization apply to all formats, but I'd be surprised to find a sensor 
outputting limited range for RGB.

Have you been able to check whether the sensor outputs limited range of full 
range YUV ? If it outputs full range you can hardcode quantization to 
V4L2_QUANTIZATION_FULL_RANGE for all formats.

> >>info->format = *fmt;
> >>
> >>return 0;

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5 00/26] Fix watchdog on Renesas R-Car Gen2 and RZ/G1

[Simon Horman]

On Wed, Feb 21, 2018 at 05:30:12PM +0100, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Wed, Feb 21, 2018 at 5:13 PM, Simon Horman  wrote:
> > On Tue, Feb 20, 2018 at 01:51:28PM +0100, Geert Uytterhoeven wrote:
> >> On Mon, Feb 12, 2018 at 6:44 PM, Fabrizio Castro
> >>  wrote:
> >> > this series has been around for some time as RFC, and it has collected
> >> > useful comments from the community along the way.
> >> > The solution proposed by this patch set works for most R-Car Gen2 and
> >> > RZ/G1 devices, but not all of them. We now know that for some R-Car
> >> > Gen2 early revisions there is no proper software fix. Anyway, no
> >> > product has been built around early revisions, but development boards
> >> > mounting early revisions (basically prototypes) are still out there.
> >> > As a result, this series isn't enabling the internal watchdog on R-Car
> >> > Gen2 boards, developers may enable it in board specific device trees
> >> > if needed.
> >> > This series has been tested by me on the iwg20d, iwg22d, Lager, Alt,
> >> > and Koelsch boards.
> >> >
> >> > The problem
> >> > ===
> >> > To deal with SMP on R-Car Gen2 and RZ/G1, we install a reset vector
> >> > to ICRAM1 and we program the [S]BAR registers so that when we turn ON
> >> > the non-boot CPUs they are redirected to the reset vector installed by
> >> > Linux in ICRAM1, and eventually they continue the execution to RAM,
> >> > where the SMP bring-up code will take care of the rest.
> >> > The content of the [S]BAR registers survives a watchdog triggered reset,
> >> > and as such after the watchdog fires the boot core will try and execute
> >> > the SMP bring-up code instead of jumping to the bootrom code.
> >> >
> >> > The fix
> >> > ===
> >> > The main strategy for the solution is to let the reset vector decide
> >> > if it needs to jump to shmobile_boot_fn or to the bootrom code.
> >> > In a watchdog triggered reset scenario, since the [S]BAR registers keep
> >> > their values, the boot CPU will jump into the newly designed reset
> >> > vector, the assembly routine will eventually test WOVF (a bit in register
> >> > RWTCSRA that indicates if the watchdog counter has overflown, the value
> >> > of this bit gets retained in this scenario), and jump to the bootrom code
> >> > which will in turn load up the bootloader, etc.
> >> > When bringing up SMP or using CPU hotplug, the reset vector will jump
> >> > to shmobile_boot_fn instead.
> >> >
> >> > Thank you All for your help.
> >> >
> >> > Best regards,
> >> >
> >> > Fabrizio Castro (26):
> >> >   ARM: shmobile: Add watchdog support
> >> >   ARM: dts: r8a7743: Adjust SMP routine size
> >> >   ARM: dts: r8a7745: Adjust SMP routine size
> >> >   ARM: dts: r8a7790: Adjust SMP routine size
> >> >   ARM: dts: r8a7791: Adjust SMP routine size
> >> >   ARM: dts: r8a7792: Adjust SMP routine size
> >> >   ARM: dts: r8a7793: Adjust SMP routine size
> >> >   ARM: dts: r8a7794: Adjust SMP routine size
> >> >   soc: renesas: rcar-rst: Enable watchdog as reset trigger for Gen2
> >> >   ARM: shmobile: rcar-gen2: Add watchdog support
> >> >   dt-bindings: watchdog: renesas-wdt: Add R-Car Gen2 support
> >> >   watchdog: renesas_wdt: Add R-Car Gen2 support
> >> >   watchdog: renesas_wdt: Add restart handler
> >> >   ARM: shmobile: defconfig: Enable RENESAS_WDT_GEN
> >> >   clk: renesas: r8a7743: Add rwdt clock
> >> >   clk: renesas: r8a7745: Add rwdt clock
> >> >   clk: renesas: r8a7790: Add rwdt clock
> >> >   clk: renesas: r8a7791/r8a7793: Add rwdt clock
> >> >   clk: renesas: r8a7794: Add rwdt clock
> >> >   ARM: dts: r8a7743: Add watchdog support to SoC dtsi
> >> >   ARM: dts: r8a7745: Add watchdog support to SoC dtsi
> >> >   ARM: dts: r8a7790: Add watchdog support to SoC dtsi
> >> >   ARM: dts: r8a7791: Add watchdog support to SoC dtsi
> >> >   ARM: dts: r8a7794: Add watchdog support to SoC dtsi
> >> >   ARM: dts: iwg20m: Add watchdog support to SoM dtsi
> >> >   ARM: dts: iwg22m: Add watchdog support to SoM dtsi
> >> >
> >> >  .../devicetree/bindings/watchdog/renesas-wdt.txt   | 19 ++--
> >> >  arch/arm/boot/dts/r8a7743-iwg20m.dtsi  |  5 ++
> >> >  arch/arm/boot/dts/r8a7743.dtsi | 12 -
> >> >  arch/arm/boot/dts/r8a7745-iwg22m.dtsi  |  5 ++
> >> >  arch/arm/boot/dts/r8a7745.dtsi | 12 -
> >> >  arch/arm/boot/dts/r8a7790.dtsi | 12 -
> >> >  arch/arm/boot/dts/r8a7791.dtsi | 12 -
> >> >  arch/arm/boot/dts/r8a7792.dtsi |  2 +-
> >> >  arch/arm/boot/dts/r8a7793.dtsi |  2 +-
> >> >  arch/arm/boot/dts/r8a7794.dtsi | 12 -
> >> >  arch/arm/configs/shmobile_defconfig|  1 +
> >> >  arch/arm/mach-shmobile/common.h|  6 +++
> >> >  arch/arm/mach-shmobile/headsmp.S   | 55 
> >> > ++
> >> >  

Re: [PATCH v2 19/19] ARM64: dts: r8a77965: Add EtherAVB device node

[Simon Horman]

On Wed, Feb 21, 2018 at 08:53:53PM +0300, Sergei Shtylyov wrote:
> On 02/21/2018 08:38 PM, Sergei Shtylyov wrote:

...

> >> +  clocks = < CPG_MOD 812>;
> >> +  power-domains = < 32>;
> >> +  resets = < 812>;
> >> +  phy-mode = "rgmii-txid";
> >
> >Why not just "rgmii"? TX delay is a board specific detail, no?
> >
>  I admit I took this one straight from r8a7796 dtsi.
>  Would you like to me resend and change this?
> >>>
> >>>Yes, unless Simon would fix it while merging...
> >>
> >> Can I confirm the desired change is s/rgmii-txid/rgmii/ ?
> > 
> >Yes.
> 
>Apparently that means that this prop should be overridden in the board file
> (which may not be an easy task given the board is Salvator-XS again).
> 
> [...]

Can we override it in r8a77965-salvator-x.dts or r8a77965-salvator-xs.dts?

I feel that I'm missing an important point here.

Re: [PATCH v2] videodev2.h: add helper to validate colorspace

[Laurent Pinchart]

Hi Hans,

On Tuesday, 20 February 2018 10:37:22 EET Hans Verkuil wrote:
> On 02/19/2018 11:28 PM, Niklas Söderlund wrote:
> > Hi Hans,
> > 
> > Thanks for your feedback.
> > 
> > [snip]
> > 
> >> Can you then fix v4l2-compliance to stop testing colorspace
> >> against 0xff
> >> ?
> > 
> > For now I can simply relax this test for subdevs with sources and
> > sinks.
>  
>  You also need to relax it for video nodes with MC drivers, as the DMA
>  engines don't care about colorspaces.
> >>> 
> >>> Yes, they do. Many DMA engines can at least do RGB <-> YUV conversions,
> >>> so they should get the colorspace info from their source and pass it on
> >>> to userspace (after correcting for any conversions done by the DMA
> >>> engine).
> >> 
> >> Not in the MC case. Video nodes there only model the DMA engine, and are
> >> thus not aware of colorspaces. What MC drivers do is check at stream on
> >> time when validating the pipeline that the colorspace set by userspace
> >> on the video node corresponds to the colorspace on the source pad of the
> >> connected subdev, but that's only to ensure that userspace gets a
> >> coherent view of colorspace across the pipeline, not to program the
> >> hardware. There could be exceptions, but in the general case, the video
> >> node implementation of an MC driver will accept any colorspace and only
> >> validate it at stream on time, similarly to how it does for the frame
> >> size format instance (and in the frame size case it will usually enforce
> >> min/max limits when the DMA engine limits the frame size).> 
> > I'm afraid the issue described above by Laurent is what sparked me to
> > write this commit to begin with. In my never ending VIN Gen3 patch-set I
> > currency need to carry a patch [1] to implement a hack to make sure
> > v4l2-compliance do not fail for the VIN Gen3 MC-centric use-case. This
> > patch was an attempt to be able to validate the colorspace using the
> > magic value 0xff.
> 
> This is NOT a magic value. The test that's done here is to memset the
> format structure with 0xff, then call the ioctl. Afterwards it checks
> if there are any remaining 0xff bytes left in the struct since it expects
> the driver to have overwritten it by something else. That's where the 0xff
> comes from.

It's no less or more magic than using 0xdeadbeef or any fixed value :-) I 
think we all agree that it isn't a value that is meant to be handled 
specifically by drivers, so it's not magic in that sense.

> > I don't feel strongly for this patch in particular and I'm happy to drop
> > it.  But I would like to receive some guidance on how to then properly
> > be able to handle this problem for the MC-centric VIN driver use-case.
> > One option is as you suggested to relax the test in v4l-compliance to
> > not check colorspace, but commit [2] is not enough to resolve the issue
> > for my MC use-case.
> > 
> > As Laurent stated above, the use-case is that the video device shall
> > accept any colorspace set from user-space. This colorspace is then only
> > used as stream on time to validate the MC pipeline. The VIN driver do
> > not care about colorspace, but I care about not breaking v4l2-compliance
> > as I find it's a very useful tool :-)
> 
> I think part of my confusion here is that there are two places where you
> deal with colorspaces in a DMA engine: first there is a input pad of the
> DMA engine entity, secondly there is the v4l2_pix_format for the memory
> description.
> 
> The second is set by the driver based on what userspace specified for the
> input pad, together with any changes due to additional conversions such
> as quantization range and RGB <-> YUV by the DMA engine.

No, I'm sorry, for MC-based drivers this isn't correct. The media entity that 
symbolizes the DMA engine indeed has a sink pad, but it's a video node, not a 
subdev. It thus has no media bus format configured for its sink pad. The 
closest pad in the pipeline that has a media bus format is the source pad of 
the subdev connected to the video node.

There's no communication within the kernel at G/S_FMT time between the video 
node and its connected subdev. The only time we look at the pipeline as a 
whole is when starting the stream to validate that the pipeline is correctly 
configured. We thus have to implement G/S_FMT on the video node without any 
knowledge about the connected subdev, and thus accept any colorspace.

> So any colorspace validation is done for the input pad. The question is
> what that validation should be. It's never been defined.

No format is set on the video node's entity sink pad for the reason above, so 
no validation occurs when setting the colorspace on the sink pad as that never 
happens.

> Also the handling of COLORSPACE_DEFAULT for pad formats needs to be defined.
> 
> This is not the first time this cropped up, see e.g. this RFC patch:
> 
> https://patchwork.linuxtv.org/patch/41734/
> 
> > I'm basing the following on the latest 

Re: [PATCH v4 01/16] dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings

[Laurent Pinchart]

Hi Sergei,

On Wednesday, 21 February 2018 10:35:13 EET Sergei Shtylyov wrote:
> On 2/21/2018 2:10 AM, Laurent Pinchart wrote:
> > The Renesas R-Car Gen2 and Gen3 SoCs have internal LVDS encoders. Add
> > corresponding device tree bindings.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> > Reviewed-by: Rob Herring 
> > ---
> > Changes since v1:
> > 
> > - Move the SoC name before the IP name in compatible strings
> > - Rename parallel input to parallel RGB input
> > - Fixed "renesas,r8a7743-lvds" description
> > - Document the resets property
> > - Fixed typo
> > ---
> > 
> >   .../bindings/display/bridge/renesas,lvds.txt   | 56 
> >   MAINTAINERS|  1 +
> >   2 files changed, 57 insertions(+)
> >   create mode 100644
> >   Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt> 
> > diff --git
> > a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> > b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt new
> > file mode 100644
> > index ..2b19ce51ec07
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> > @@ -0,0 +1,56 @@
> > +Renesas R-Car LVDS Encoder
> > +==
> > +
> > +These DT bindings describe the LVDS encoder embedded in the Renesas R-Car
> > +Gen2, R-Car Gen3 and RZ/G SoCs.
> > +
> > +Required properties:
> > +
> > +- compatible : Shall contain one of
> > +  - "renesas,r8a7743-lvds" for R8A7743 (RZ/G1M) compatible LVDS encoders
> > +  - "renesas,r8a7790-lvds" for R8A7790 (R-Car H2) compatible LVDS
> > encoders
> > +  - "renesas,r8a7791-lvds" for R8A7791 (R-Car M2-W) compatible LVDS
> > encoders +  - "renesas,r8a7793-lvds" for R8A7791 (R-Car M2-N) compatible
> > LVDS encoders +  - "renesas,r8a7795-lvds" for R8A7795 (R-Car H3)
> > compatible LVDS encoders +  - "renesas,r8a7796-lvds" for R8A7796 (R-Car
> > M3-W) compatible LVDS encoders +
> > +- reg: Base address and length for the memory-mapped registers
> > +- clocks: A phandle + clock-specifier pair for the functional clock
> > +- resets: A phandle + reset specifier for the module reset
> > +
> > +Required nodes:
> > +
> > +The LVDS encoder has two video ports. Their connections are modelled
> > using the +OF graph bindings specified in
> > Documentation/devicetree/bindings/graph.txt. +
> > +- Video port 0 corresponds to the parallel RGB input
> > +- Video port 1 corresponds to the LVDS output
> > +
> > +Each port shall have a single endpoint.
> > +
> > +
> > +Example:
> > +
> > +   lvds0: lvds@feb9 {
> 
> "lvds-encoder@feb9" maybe?

Or just "encoder@feb9" ? Or "display-encoder@feb9" ? Rob, any 
preference ?

> And do we need a # in the label if the encoder is alone in DT anyway?

As a matter of fact r8a7790, on which this example is based, has two LVDS 
encoders. I've only included one in the example as adding a second one 
wouldn't have added any value.

> [...]

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 19/19] ARM64: dts: r8a77965: Add EtherAVB device node

[Sergei Shtylyov]

On 02/21/2018 09:23 PM, Simon Horman wrote:

> ...
> 
 +  clocks = < CPG_MOD 812>;
 +  power-domains = < 32>;
 +  resets = < 812>;
 +  phy-mode = "rgmii-txid";
>>>
>>>Why not just "rgmii"? TX delay is a board specific detail, no?
>>>
>> I admit I took this one straight from r8a7796 dtsi.
>> Would you like to me resend and change this?
>
>Yes, unless Simon would fix it while merging...

 Can I confirm the desired change is s/rgmii-txid/rgmii/ ?
>>>
>>>Yes.
>>
>>Apparently that means that this prop should be overridden in the board 
>> file
>> (which may not be an easy task given the board is Salvator-XS again).
>>
>> [...]
> 
> Can we override it in r8a77965-salvator-x.dts or r8a77965-salvator-xs.dts?

   In salvator-common.dtsi most probably -- it has the PHY data for Ether AVB.

> I feel that I'm missing an important point here.

   Well, r8a779{5|6}.dtsi also have phy-mode = "rgmii-txid" (which was 
unjustified
in my current understanding). Thus such board override wouldn't hurt them. But 
we lack
a patch modifying salvator-common.dtsi in htis series, so I'm now thinking a 
respin of
this series is needed anyway... sorry for being unclear. :-)

MBR, Sergei

Re: [PATCH V3] ARM: shmobile: stout: enable R-Car Gen2 regulator quirk

[Simon Horman]

On Thu, Feb 15, 2018 at 12:33:50PM +0100, Marek Vasut wrote:
> Regulator setup is suboptimal on H2 Stout too. The Stout newly has
> two DA9210 regulators, so the quirk is extended to handle another
> DA9210 at i2c address 0x70.
> 
> Signed-off-by: Marek Vasut 
> Cc: Geert Uytterhoeven 
> Cc: Kuninori Morimoto 
> Cc: Simon Horman 
> Cc: Wolfram Sang 
> ---
> V2: - Handle another DA9210 at 0x70
> - Drop explicit board list from the leading comment in the file
> V3: - Correct da9xxx_msgs[] array size
> - Send messages to all three regulators on Stout only

Thanks, applied.

Re: [PATCH v2 19/19] ARM64: dts: r8a77965: Add EtherAVB device node

[Sergei Shtylyov]

On 02/21/2018 08:38 PM, Sergei Shtylyov wrote:

>> Populate the ethernet@e680 device node to enable Ethernet interface
>> for R-Car M3-N (r8a77965) SoC.
>>
>> Signed-off-by: Jacopo Mondi 
>> Reviewed-by: Geert Uytterhoeven 
>>
>> ---
>> v1 -> v2:
>> - Replace ALWAYS_ON power area identifier with numeric constant
>> ---
>>  arch/arm64/boot/dts/renesas/r8a77965.dtsi | 43 
>> ++-
>>  1 file changed, 42 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi 
>> b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
>> index 55f05f7..c249895 100644
>> --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
>> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
>> @@ -520,7 +520,48 @@
>>  };
>>
>>  avb: ethernet@e680 {
>> -/* placeholder */
>> +compatible = "renesas,etheravb-r8a77965",
>> + "renesas,etheravb-rcar-gen3";
>> +reg = <0 0xe680 0 0x800>, <0 0xe6a0 0 
>> 0x1>;
>> +interrupts = ,
>> + ,
>> + ,
>> + ,
>> + ,
>> + ,
>> + ,
>> + ,
>> + ,
>> + ,
>> + ,
>> + ,
>> + ,
>> + ,
>> + ,
>> + ,
>> + ,
>> + ,
>> + ,
>> + ,
>> + ,
>> + ,
>> + ,
>> + ,
>> + ;
>> +interrupt-names = "ch0", "ch1", "ch2", "ch3",
>> +  "ch4", "ch5", "ch6", "ch7",
>> +  "ch8", "ch9", "ch10", "ch11",
>> +  "ch12", "ch13", "ch14", 
>> "ch15",
>> +  "ch16", "ch17", "ch18", 
>> "ch19",
>> +  "ch20", "ch21", "ch22", 
>> "ch23",
>> +  "ch24";
>> +clocks = < CPG_MOD 812>;
>> +power-domains = < 32>;
>> +resets = < 812>;
>> +phy-mode = "rgmii-txid";
>
>Why not just "rgmii"? TX delay is a board specific detail, no?
>
 I admit I took this one straight from r8a7796 dtsi.
 Would you like to me resend and change this?
>>>
>>>Yes, unless Simon would fix it while merging...
>>
>> Can I confirm the desired change is s/rgmii-txid/rgmii/ ?
> 
>Yes.

   Apparently that means that this prop should be overridden in the board file
(which may not be an easy task given the board is Salvator-XS again).

[...]

MBR, Sergei

[PATCH v10 00/10] Renesas Capture Engine Unit (CEU) V4L2 driver

[Jacopo Mondi]

Hello,
   10th round, one step closer to finalize CEU inclusion (hopefully).

I have dropped patch 11/11 of v9 as it is still a debated topic and I would
like to have it clarified without blocking this series.

In patch [3/10] Hans noticed that after changing input, format should be set on
CEU again to make sure userspace can start streaming operations. While
there's no clear preference if applying default formats to the newly selected
input is the best option, I went for this one. While at there I renamed the
"ceu_init_formats()" function to "ceu_init_mbus_fmt()" and moved field
selection to "ceu_set[_default]_fmt()" functions.

In patch [7/10] I have dropped two redundant memsets and closed a few warnings
received from 0-day running sparse on the patch. I have declared two fields as
static, and transformed an "if" clause in a "switch" one, to catch all possible
cases with a "default" one and while at there replaced the frame interval array
size definition with an explicit ARRAY_SIZE().

Apart from that, all patches are now acked/reviewed, apart from 03/10 that has
Hans' ack still pending.

Diff from v9 is so short, I captured it here for the interested ones:
https://paste.debian.net/hidden/91567755

Thanks
   j

v9->v10:
- Close 0-days warning on ov772x frame interval
- Set default format on CEU after input change
- Drop ov7670 mbus frame format set not to block this series while topic
  gets clarified

v8->v9:
- Address Laurent's review of ov772x frame rate
- Address Sergei comment on ceu node name

v7->v8:
- Calculate PLL divider/multiplier and do not use static tables
- Change RZ/A1-H to RZ/A1H (same for L) in bindings documentation
- Use rounded clock rate in Migo-R board code as SH clk_set_clk()
  implementation does not perform rounding
- Set ycbcr_enc and other fields of v4l2_mbus_format for ov772x as patch
  [11/11] does for ov7670

v6->v7:
- Add patch to handle ycbr_enc and other fields of v4l2_mbus_format for ov7670
- Add patch to handle frame interval for ov772x
- Rebased on Hans' media-tree/parm branch with v4l2_g/s_parm_cap
- Drop const modifier in CEU releated fields of Migo-R setup.c board file
  to silence complier warnings.

v5->v6:
- Add Hans' Acked-by to most patches
- Fix a bad change in ov772x get_selection
- Add .buf_prepare callack to CEU and verify plane sizes there
- Remove VB2_USERPTR from supported io_modes in CEU driver
- Remove read() fops in CEU driver

v4->v5:
- Added Rob's and Laurent's Reviewed-by tag to DT bindings
- Change CEU driver module license to "GPL v2" to match SPDX identifier as
  suggested by Philippe Ombredanne
- Make struct ceu_data static as suggested by Laurent and add his
  Reviewed-by to CEU driver.

v3->v4:
- Drop generic fallback compatible string "renesas,ceu"
- Addressed Laurent's comments on [3/9]
  - Fix error messages on irq get/request
  - Do not leak ceudev if irq_get fails
  - Make irq_mask a const field

v2->v3:
- Improved DT bindings removing standard properties (pinctrl- ones and
  remote-endpoint) not specific to this driver and improved description of
  compatible strings
- Remove ov772x's xlkc_rate property and set clock rate in Migo-R board file
- Made 'xclk' clock private to ov772x driver in Migo-R board file
- Change 'rstb' GPIO active output level and changed ov772x and tw9910 drivers
  accordingly as suggested by Fabio
- Minor changes in CEU driver to address Laurent's comments
- Moved Migo-R setup patch to the end of the series to silence 0-day bot
- Renamed tw9910 clock to 'xti' as per video decoder manual
- Changed all SPDX identifiers to GPL-2.0 from previous GPL-2.0+

v1->v2:
 - DT
 -- Addressed Geert's comments and added clocks for CEU to mstp6 clock source
 -- Specified supported generic video iterfaces properties in dt-bindings and
simplified example

 - CEU driver
 -- Re-worked interrupt handler, interrupt management, reset(*) and capture
start operation
 -- Re-worked querycap/enum_input/enum_frameintervals to fix some
v4l2_compliance failures
 -- Removed soc_camera legacy operations g/s_mbus_format
 -- Update to new notifier implementation
 -- Fixed several comments from Hans, Laurent and Sakari

 - Migo-R
 -- Register clocks and gpios for sensor drivers in Migo-R setup
 -- Updated sensors (tw9910 and ov772x) drivers headers and drivers to close
remarks from Hans and Laurent:
 --- Removed platform callbacks and handle clocks and gpios from sensor drivers
 --- Remove g/s_mbus_config operations

Jacopo Mondi (10):
  dt-bindings: media: Add Renesas CEU bindings
  include: media: Add Renesas CEU driver interface
  media: platform: Add Renesas CEU driver
  ARM: dts: r7s72100: Add Capture Engine Unit (CEU)
  media: i2c: Copy ov772x soc_camera sensor driver
  media: i2c: ov772x: Remove soc_camera dependencies
  media: i2c: ov772x: Support frame interval handling
  media: i2c: Copy tw9910 soc_camera sensor driver
  media: i2c: tw9910: Remove soc_camera dependencies
  arch: sh: migor: Use new renesas-ceu camera 

[PATCH v10 01/10] 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..3fc66df
--- /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/A1H
+  and RZ/A1M 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 v10 02/10] 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 v10 03/10] media: 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 | 1670 ++
 3 files changed, 1680 insertions(+)
 create mode 100644 drivers/media/platform/renesas-ceu.c

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 614fbef..f9cc058 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 7f30804..85e1121 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..6624fba
--- /dev/null
+++ b/drivers/media/platform/renesas-ceu.c
@@ -0,0 +1,1670 @@
+// 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 
+#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 

[PATCH v10 04/10] 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..23e05ce 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: camera@e821 {
+   reg = <0xe821 0x3000>;
+   compatible = "renesas,r7s72100-ceu";
+   interrupts = ;
+   clocks = <_clks R7S72100_CLK_CEU>;
+   power-domains = <_clocks>;
+   status = "disabled";
+   };
 };
-- 
2.7.4

[PATCH v10 07/10] media: i2c: ov772x: Support frame interval handling

[Jacopo Mondi]

Add support to ov772x driver for frame intervals handling and enumeration.
Tested with 10MHz and 24MHz input clock at VGA and QVGA resolutions for
10, 15 and 30 frame per second rates.

Signed-off-by: Jacopo Mondi 
Reviewed-by: Laurent Pinchart 
Acked-by: Hans Verkuil 
---
 drivers/media/i2c/ov772x.c | 213 +
 1 file changed, 196 insertions(+), 17 deletions(-)

diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 23106d7..3edf0cb 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -250,6 +250,7 @@
 #define AEC_1p2 0x10   /*  01: 1/2  window */
 #define AEC_1p4 0x20   /*  10: 1/4  window */
 #define AEC_2p3 0x30   /*  11: Low 2/3 window */
+#define COM4_RESERVED   0x01   /* Reserved bit */

 /* COM5 */
 #define AFR_ON_OFF  0x80   /* Auto frame rate control ON/OFF selection */
@@ -267,6 +268,10 @@
/* AEC max step control */
 #define AEC_NO_LIMIT0x01   /*   0 : AEC incease step has limit */
/*   1 : No limit to AEC increase step */
+/* CLKRC */
+   /* Input clock divider register */
+#define CLKRC_RESERVED  0x80   /* Reserved bit */
+#define CLKRC_DIV(n)((n) - 1)

 /* COM7 */
/* SCCB Register Reset */
@@ -373,6 +378,19 @@
 #define VERSION(pid, ver) ((pid<<8)|(ver&0xFF))

 /*
+ * PLL multipliers
+ */
+static struct {
+   unsigned int mult;
+   u8 com4;
+} ov772x_pll[] = {
+   { 1, PLL_BYPASS, },
+   { 4, PLL_4x, },
+   { 6, PLL_6x, },
+   { 8, PLL_8x, },
+};
+
+/*
  * struct
  */

@@ -388,6 +406,7 @@ struct ov772x_color_format {
 struct ov772x_win_size {
char *name;
unsigned char com7_bit;
+   unsigned int  sizeimage;
struct v4l2_rect  rect;
 };

@@ -404,6 +423,7 @@ struct ov772x_priv {
unsigned shortflag_hflip:1;
/* band_filter = COM8[5] ? 256 - BDBASE : 0 */
unsigned shortband_filter;
+   unsigned int  fps;
 };

 /*
@@ -487,27 +507,34 @@ static const struct ov772x_color_format ov772x_cfmts[] = {

 static const struct ov772x_win_size ov772x_win_sizes[] = {
{
-   .name = "VGA",
-   .com7_bit = SLCT_VGA,
+   .name   = "VGA",
+   .com7_bit   = SLCT_VGA,
+   .sizeimage  = 510 * 748,
.rect = {
-   .left = 140,
-   .top = 14,
-   .width = VGA_WIDTH,
-   .height = VGA_HEIGHT,
+   .left   = 140,
+   .top= 14,
+   .width  = VGA_WIDTH,
+   .height = VGA_HEIGHT,
},
}, {
-   .name = "QVGA",
-   .com7_bit = SLCT_QVGA,
+   .name   = "QVGA",
+   .com7_bit   = SLCT_QVGA,
+   .sizeimage  = 278 * 576,
.rect = {
-   .left = 252,
-   .top = 6,
-   .width = QVGA_WIDTH,
-   .height = QVGA_HEIGHT,
+   .left   = 252,
+   .top= 6,
+   .width  = QVGA_WIDTH,
+   .height = QVGA_HEIGHT,
},
},
 };

 /*
+ * frame rate settings lists
+ */
+static unsigned int ov772x_frame_intervals[] = { 5, 10, 15, 20, 30, 60 };
+
+/*
  * general function
  */

@@ -574,6 +601,128 @@ static int ov772x_s_stream(struct v4l2_subdev *sd, int 
enable)
return 0;
 }

+static int ov772x_set_frame_rate(struct ov772x_priv *priv,
+struct v4l2_fract *tpf,
+const struct ov772x_color_format *cfmt,
+const struct ov772x_win_size *win)
+{
+   struct i2c_client *client = v4l2_get_subdevdata(>subdev);
+   unsigned long fin = clk_get_rate(priv->clk);
+   unsigned int fps = tpf->numerator ?
+  tpf->denominator / tpf->numerator :
+  tpf->denominator;
+   unsigned int best_diff;
+   unsigned int fsize;
+   unsigned int pclk;
+   unsigned int diff;
+   unsigned int idx;
+   unsigned int i;
+   u8 clkrc = 0;
+   u8 com4 = 0;
+   int ret;
+
+   /* Approximate to the closest supported frame interval. */
+   best_diff = ~0L;
+   for (i = 0, idx = 0; i < ARRAY_SIZE(ov772x_frame_intervals); i++) {
+   diff = abs(fps - ov772x_frame_intervals[i]);
+   if (diff < best_diff) {
+   idx = i;
+   best_diff = diff;
+   }
+   }

[PATCH v10 06/10] 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 | 172 ++---
 include/media/i2c/ov772x.h |   6 +-
 4 files changed, 133 insertions(+), 57 deletions(-)

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 9f18cd2..e71e968 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 c0f94cd..b0489a1 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -68,6 +68,7 @@ obj-$(CONFIG_VIDEO_OV5670) += ov5670.o
 obj-$(CONFIG_VIDEO_OV6650) += ov6650.o
 obj-$(CONFIG_VIDEO_OV7640) += ov7640.o
 obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
+obj-$(CONFIG_VIDEO_OV772X) += ov772x.o
 obj-$(CONFIG_VIDEO_OV7740) += ov7740.o
 obj-$(CONFIG_VIDEO_OV9650) += ov9650.o
 obj-$(CONFIG_VIDEO_OV13858) += ov13858.o
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 8063835..23106d7 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 v10 05/10] media: 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 v10 08/10] media: 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 v10 09/10] 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 e71e968..0460613 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 b0489a1..23c3ac6 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)
+{
+   

[PATCH v10 10/10] arch: sh: migor: Use new renesas-ceu camera driver

[Jacopo Mondi]

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 
---
 arch/sh/boards/mach-migor/setup.c  | 225 +++--
 arch/sh/kernel/cpu/sh4a/clock-sh7722.c |   2 +-
 2 files changed, 101 insertions(+), 126 deletions(-)

diff --git a/arch/sh/boards/mach-migor/setup.c 
b/arch/sh/boards/mach-migor/setup.c
index 0bcbe58..271dfc2 100644
--- a/arch/sh/boards/mach-migor/setup.c
+++ b/arch/sh/boards/mach-migor/setup.c
@@ -1,17 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * Renesas System Solutions Asia Pte. Ltd - Migo-R
  *
  * Copyright (C) 2008 Magnus Damm
- *
- * This file is subject to the terms and conditions of the GNU General Public
- * License.  See the file "COPYING" in the main directory of this archive
- * for more details.
  */
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -23,10 +22,11 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
@@ -45,6 +45,9 @@
  * 0x1800   8GB8   NAND Flash (K9K8G08U0A)
  */
 
+#define CEU_BUFFER_MEMORY_SIZE (4 << 20)
+static phys_addr_t ceu_dma_membase;
+
 static struct smc91x_platdata smc91x_info = {
.flags = SMC91X_USE_16BIT | SMC91X_NOWAIT,
 };
@@ -301,65 +304,24 @@ static struct platform_device migor_lcdc_device = {
},
 };
 
-static struct clk *camera_clk;
-static DEFINE_MUTEX(camera_lock);
-
-static void camera_power_on(int is_tw)
-{
-   mutex_lock(_lock);
-
-   /* Use 10 MHz VIO_CKO instead of 24 MHz to work
-* around signal quality issues on Panel Board V2.1.
-*/
-   camera_clk = clk_get(NULL, "video_clk");
-   clk_set_rate(camera_clk, 1000);
-   clk_enable(camera_clk); /* start VIO_CKO */
-
-   /* use VIO_RST to take camera out of reset */
-   mdelay(10);
-   if (is_tw) {
-   gpio_set_value(GPIO_PTT2, 0);
-   gpio_set_value(GPIO_PTT0, 0);
-   } else {
-   gpio_set_value(GPIO_PTT0, 1);
-   }
-   gpio_set_value(GPIO_PTT3, 0);
-   mdelay(10);
-   gpio_set_value(GPIO_PTT3, 1);
-   mdelay(10); /* wait to let chip come out of reset */
-}
-
-static void camera_power_off(void)
-{
-   clk_disable(camera_clk); /* stop VIO_CKO */
-   clk_put(camera_clk);
-
-   gpio_set_value(GPIO_PTT3, 0);
-   mutex_unlock(_lock);
-}
-
-static int ov7725_power(struct device *dev, int mode)
-{
-   if (mode)
-   camera_power_on(0);
-   else
-   camera_power_off();
-
-   return 0;
-}
-
-static int tw9910_power(struct device *dev, int mode)
-{
-   if (mode)
-   camera_power_on(1);
-   else
-   camera_power_off();
-
-   return 0;
-}
-
-static struct sh_mobile_ceu_info sh_mobile_ceu_info = {
-   .flags = SH_CEU_FLAG_USE_8BIT_BUS,
+static struct ceu_platform_data ceu_pdata = {
+   .num_subdevs= 2,
+   .subdevs = {
+   { /* [0] = ov772x */
+   .flags  = 0,
+   .bus_width  = 8,
+   .bus_shift  = 0,
+   .i2c_adapter_id = 0,
+   .i2c_address= 0x21,
+   },
+   { /* [1] = tw9910 */
+   .flags  = 0,
+   .bus_width  = 8,
+   .bus_shift  = 0,
+   .i2c_adapter_id = 0,
+   .i2c_address= 0x45,
+   },
+   },
 };
 
 static struct resource migor_ceu_resources[] = {
@@ -373,18 +335,32 @@ static struct resource migor_ceu_resources[] = {
.start  = evt2irq(0x880),
.flags  = IORESOURCE_IRQ,
},
-   [2] = {
-   /* place holder for contiguous memory */
-   },
 };
 
 static struct platform_device migor_ceu_device = {
-   .name   = "sh_mobile_ceu",
-   .id = 0, /* "ceu0" clock */
+   .name   = "renesas-ceu",
+   .id = 0, /* ceu.0 */
.num_resources  = ARRAY_SIZE(migor_ceu_resources),
.resource   = migor_ceu_resources,
.dev= {
-   

Re: [PATCH v2 19/19] ARM64: dts: r8a77965: Add EtherAVB device node

[Sergei Shtylyov]

On 02/21/2018 08:31 PM, Simon Horman wrote:

> Populate the ethernet@e680 device node to enable Ethernet interface
> for R-Car M3-N (r8a77965) SoC.
>
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Geert Uytterhoeven 
>
> ---
> v1 -> v2:
> - Replace ALWAYS_ON power area identifier with numeric constant
> ---
>  arch/arm64/boot/dts/renesas/r8a77965.dtsi | 43 
> ++-
>  1 file changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi 
> b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> index 55f05f7..c249895 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> @@ -520,7 +520,48 @@
>   };
>
>   avb: ethernet@e680 {
> - /* placeholder */
> + compatible = "renesas,etheravb-r8a77965",
> +  "renesas,etheravb-rcar-gen3";
> + reg = <0 0xe680 0 0x800>, <0 0xe6a0 0 0x1>;
> + interrupts = ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ;
> + interrupt-names = "ch0", "ch1", "ch2", "ch3",
> +   "ch4", "ch5", "ch6", "ch7",
> +   "ch8", "ch9", "ch10", "ch11",
> +   "ch12", "ch13", "ch14", "ch15",
> +   "ch16", "ch17", "ch18", "ch19",
> +   "ch20", "ch21", "ch22", "ch23",
> +   "ch24";
> + clocks = < CPG_MOD 812>;
> + power-domains = < 32>;
> + resets = < 812>;
> + phy-mode = "rgmii-txid";

Why not just "rgmii"? TX delay is a board specific detail, no?

>>> I admit I took this one straight from r8a7796 dtsi.
>>> Would you like to me resend and change this?
>>
>>Yes, unless Simon would fix it while merging...
> 
> Can I confirm the desired change is s/rgmii-txid/rgmii/ ?

   Yes.

> If so I can fix that up.
 
   Thank you!

MBR, Sergei

Re: [PATCH v2 19/19] ARM64: dts: r8a77965: Add EtherAVB device node

[Simon Horman]

On Wed, Feb 21, 2018 at 06:48:59PM +0300, Sergei Shtylyov wrote:
> Hello!
> 
> On 02/21/2018 01:07 PM, jacopo mondi wrote:
> 
> >>> Populate the ethernet@e680 device node to enable Ethernet interface
> >>> for R-Car M3-N (r8a77965) SoC.
> >>>
> >>> Signed-off-by: Jacopo Mondi 
> >>> Reviewed-by: Geert Uytterhoeven 
> >>>
> >>> ---
> >>> v1 -> v2:
> >>> - Replace ALWAYS_ON power area identifier with numeric constant
> >>> ---
> >>>  arch/arm64/boot/dts/renesas/r8a77965.dtsi | 43 
> >>> ++-
> >>>  1 file changed, 42 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi 
> >>> b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> >>> index 55f05f7..c249895 100644
> >>> --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> >>> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> >>> @@ -520,7 +520,48 @@
> >>>   };
> >>>
> >>>   avb: ethernet@e680 {
> >>> - /* placeholder */
> >>> + compatible = "renesas,etheravb-r8a77965",
> >>> +  "renesas,etheravb-rcar-gen3";
> >>> + reg = <0 0xe680 0 0x800>, <0 0xe6a0 0 0x1>;
> >>> + interrupts = ,
> >>> +  ,
> >>> +  ,
> >>> +  ,
> >>> +  ,
> >>> +  ,
> >>> +  ,
> >>> +  ,
> >>> +  ,
> >>> +  ,
> >>> +  ,
> >>> +  ,
> >>> +  ,
> >>> +  ,
> >>> +  ,
> >>> +  ,
> >>> +  ,
> >>> +  ,
> >>> +  ,
> >>> +  ,
> >>> +  ,
> >>> +  ,
> >>> +  ,
> >>> +  ,
> >>> +  ;
> >>> + interrupt-names = "ch0", "ch1", "ch2", "ch3",
> >>> +   "ch4", "ch5", "ch6", "ch7",
> >>> +   "ch8", "ch9", "ch10", "ch11",
> >>> +   "ch12", "ch13", "ch14", "ch15",
> >>> +   "ch16", "ch17", "ch18", "ch19",
> >>> +   "ch20", "ch21", "ch22", "ch23",
> >>> +   "ch24";
> >>> + clocks = < CPG_MOD 812>;
> >>> + power-domains = < 32>;
> >>> + resets = < 812>;
> >>> + phy-mode = "rgmii-txid";
> >>
> >>Why not just "rgmii"? TX delay is a board specific detail, no?
> >>
> > 
> > I admit I took this one straight from r8a7796 dtsi.
> > Would you like to me resend and change this?
> 
>Yes, unless Simon would fix it while merging...

Can I confirm the desired change is s/rgmii-txid/rgmii/ ?

If so I can fix that up.

Re: [PATCH v2 15/19] dt-bindings: gpio: Add support for r8a77965

[Simon Horman]

On Tue, Feb 20, 2018 at 04:12:17PM +0100, Jacopo Mondi wrote:
> Add compatible string for R-Car M3-N (r8a77965) in gpio-rcar.
> 
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Geert Uytterhoeven 

Reviewed-by: Simon Horman 

Re: [PATCH v2 16/19] ARM64: dts: r8a77965: Add GPIO nodes

[Simon Horman]

On Tue, Feb 20, 2018 at 04:12:18PM +0100, Jacopo Mondi wrote:
> Add GPIO nodes to r8a77965 SoC device tree file.
> 
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Geert Uytterhoeven 
> 
> ---
> v1 -> v2:
> - Replace ALWAYS_ON power area define with numeric constant
> - Do not move gpio nodes

Thanks, applied with prefix updated to:

arm64: dts: renesas: r8a77965:

Re: [PATCH v2 14/19] ARM64: dts: r8a77965: Add SCIF device nodes

[Simon Horman]

On Tue, Feb 20, 2018 at 04:12:16PM +0100, Jacopo Mondi wrote:
> Add SCIF[0-5] device nodes for M3-N (r8a77965) SoC.
> 
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Geert Uytterhoeven 

Thanks, applied with prefix updated to:

arm64: dts: renesas: r8a77965:

Re: [PATCH v2 11/19] ARM64: dts: r8a77965: Add dmac device nods

[Simon Horman]

On Tue, Feb 20, 2018 at 04:12:13PM +0100, Jacopo Mondi wrote:
> Add dmac[0-2] device nodes for R-Car M3-N (r8a77965) SoC.
> 
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Geert Uytterhoeven 

Thanks, applied with prefix updated to:

arm64: dts: renesas: r8a77965:

Re: [PATCH v2 12/19] dt-bindings: serial: sh-sci: Add support for r8a77965 (H)SCIF

[Simon Horman]

On Tue, Feb 20, 2018 at 04:12:14PM +0100, Jacopo Mondi wrote:
> Add documentation for r8a77965 compatible string to Renesas sci-serial
> device tree bindings documentation.
> 
> Signed-off-by: Jacopo Mondi 

Reviewed-by: Simon Horman 

Re: [PATCH v2 09/19] ARM64: dts: Add R-Car Salvator-x M3-N support

[Simon Horman]

On Tue, Feb 20, 2018 at 04:46:23PM +0100, Geert Uytterhoeven wrote:
> On Tue, Feb 20, 2018 at 4:12 PM, Jacopo Mondi  
> wrote:
> > Add basic support for R-Car Salvator-X M3-N (R8A77965) board.
> >
> > Based on original work from:
> > Takeshi Kihara 
> > Magnus Damm 
> >
> > Signed-off-by: Jacopo Mondi 
> 
> Reviewed-by: Geert Uytterhoeven 

Thanks, applied with prefix updated to

"arm64: dts: renesas:"

There was some fuzz applying this patch, the result is as follows.

From: Jacopo Mondi 
Subject: [PATCH] arm64: dts: renesas: Add R-Car Salvator-x M3-N support

Add basic support for R-Car Salvator-X M3-N (R8A77965) board.

Based on original work from:
Takeshi Kihara 
Magnus Damm 

Signed-off-by: Jacopo Mondi 
Reviewed-by: Geert Uytterhoeven 
---
 arch/arm64/boot/dts/renesas/Makefile|  1 +
 arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts | 21 +
 2 files changed, 22 insertions(+)
 create mode 100644 arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts

diff --git a/arch/arm64/boot/dts/renesas/Makefile 
b/arch/arm64/boot/dts/renesas/Makefile
index c885eef4e660..9ea5ec0daeba 100644
--- a/arch/arm64/boot/dts/renesas/Makefile
+++ b/arch/arm64/boot/dts/renesas/Makefile
@@ -7,6 +7,7 @@ dtb-$(CONFIG_ARCH_R8A7795) += r8a7795-es1-h3ulcb-kf.dtb
 dtb-$(CONFIG_ARCH_R8A7796) += r8a7796-salvator-x.dtb r8a7796-m3ulcb.dtb
 dtb-$(CONFIG_ARCH_R8A7796) += r8a7796-m3ulcb-kf.dtb
 dtb-$(CONFIG_ARCH_R8A7796) += r8a7796-salvator-xs.dtb
+dtb-$(CONFIG_ARCH_R8A77965) += r8a77965-salvator-x.dtb
 dtb-$(CONFIG_ARCH_R8A77970) += r8a77970-eagle.dtb r8a77970-v3msk.dtb
 dtb-$(CONFIG_ARCH_R8A77980) += r8a77980-condor.dtb
 dtb-$(CONFIG_ARCH_R8A77995) += r8a77995-draak.dtb
diff --git a/arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts 
b/arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts
new file mode 100644
index ..75d890d91df9
--- /dev/null
+++ b/arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Device Tree Source for the Salvator-X board with R-Car M3-N
+ *
+ * Copyright (C) 2018 Jacopo Mondi 
+ */
+
+/dts-v1/;
+#include "r8a77965.dtsi"
+#include "salvator-x.dtsi"
+
+/ {
+   model = "Renesas Salvator-X board based on r8a77965";
+   compatible = "renesas,salvator-x", "renesas,r8a77965";
+
+   memory@4800 {
+   device_type = "memory";
+   /* first 128MB is reserved for secure area. */
+   reg = <0x0 0x4800 0x0 0x7800>;
+   };
+};
-- 
2.11.0

Re: [PATCH v2 08/19] ARM64: dts: Add Renesas R8A77965 SoC support

[Simon Horman]

On Tue, Feb 20, 2018 at 04:45:48PM +0100, Geert Uytterhoeven wrote:
> On Tue, Feb 20, 2018 at 4:12 PM, Jacopo Mondi  
> wrote:
> > Basic support for the Gen 3 R-Car M3-N SoC.
> >
> > Based on original work from:
> > Takeshi Kihara 
> > Magnus Damm 
> >
> > Signed-off-by: Jacopo Mondi 
> 
> Reviewed-by: Geert Uytterhoeven 

Thanks, applied with the patch subject updated to

arm64: dts: renesas: initial R8A77965 SoC device tree

Please use "arm64: dts: renesas:" as the prefix for Gen-3 DTS/DTSI patches.

Re: [PATCH v2 07/19] ARM64: Add Renesas R-Car M3-N config symbol

[Simon Horman]

On Tue, Feb 20, 2018 at 04:12:09PM +0100, Jacopo Mondi wrote:
> Add configuration option for the R-Car M3-N (R8A77965) SoC.
> 
> Signed-off-by: Jacopo Mondi 

Thanks, I have applied this after updating the subject to:

arm64: add Renesas R8A77965 support

Please use "arm64" rather than ARM64 as a prefix.
(But "ARM" rather than "arm"!).

Re: [PATCH v2 06/19] dt-bindings: arm: Document R-Car M3-N SoC DT bindings

[Simon Horman]

On Tue, Feb 20, 2018 at 04:12:08PM +0100, Jacopo Mondi wrote:
> Add device tree bindings documentation for Renesas R-Car M3-N (r8a77965)
> SoC.
> 
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Geert Uytterhoeven 
> Reviewed-by: Rob Herring 

Thanks, this is already applied.

Re: [PATCH v2 04/19] soc: renesas: rcar-sysc: Add R-Car M3-N support

[Simon Horman]

On Tue, Feb 20, 2018 at 04:12:06PM +0100, Jacopo Mondi wrote:
> Add support for R-Car M3-N (R8A77965) power areas.
> 
> Signed-off-by: Jacopo Mondi 
> 
> ---
> v1->v2:
> - Remove A2VC0 power area
> - Add A3VP power area
> ---
>  .../bindings/power/renesas,rcar-sysc.txt   |  1 +
>  drivers/soc/renesas/Kconfig|  5 +++
>  drivers/soc/renesas/Makefile   |  1 +
>  drivers/soc/renesas/r8a77965-sysc.c| 37 
> ++
>  drivers/soc/renesas/rcar-sysc.c|  3 ++
>  drivers/soc/renesas/rcar-sysc.h|  1 +
>  include/dt-bindings/power/r8a77965-sysc.h  | 30 ++
>  7 files changed, 78 insertions(+)
>  create mode 100644 drivers/soc/renesas/r8a77965-sysc.c
>  create mode 100644 include/dt-bindings/power/r8a77965-sysc.h

Thanks, applied.

There was some fuzz. The result is below.

From: Jacopo Mondi 
Subject: [PATCH] soc: renesas: rcar-sysc: Add R-Car M3-N support

Add support for R-Car M3-N (R8A77965) power areas.

Signed-off-by: Jacopo Mondi 
Reviewed-by: Geert Uytterhoeven 
---
 .../bindings/power/renesas,rcar-sysc.txt   |  1 +
 drivers/soc/renesas/Kconfig|  5 +++
 drivers/soc/renesas/Makefile   |  1 +
 drivers/soc/renesas/r8a77965-sysc.c| 37 ++
 drivers/soc/renesas/rcar-sysc.c|  3 ++
 drivers/soc/renesas/rcar-sysc.h|  1 +
 include/dt-bindings/power/r8a77965-sysc.h  | 30 ++
 7 files changed, 78 insertions(+)
 create mode 100644 drivers/soc/renesas/r8a77965-sysc.c
 create mode 100644 include/dt-bindings/power/r8a77965-sysc.h

diff --git a/Documentation/devicetree/bindings/power/renesas,rcar-sysc.txt 
b/Documentation/devicetree/bindings/power/renesas,rcar-sysc.txt
index 6284a9550b3c..ab399e559257 100644
--- a/Documentation/devicetree/bindings/power/renesas,rcar-sysc.txt
+++ b/Documentation/devicetree/bindings/power/renesas,rcar-sysc.txt
@@ -17,6 +17,7 @@ Required properties:
   - "renesas,r8a7794-sysc" (R-Car E2)
   - "renesas,r8a7795-sysc" (R-Car H3)
   - "renesas,r8a7796-sysc" (R-Car M3-W)
+  - "renesas,r8a77965-sysc" (R-Car M3-N)
   - "renesas,r8a77970-sysc" (R-Car V3M)
   - "renesas,r8a77980-sysc" (R-Car V3H)
   - "renesas,r8a77995-sysc" (R-Car D3)
diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig
index 6caa393e3a2d..7106a6330210 100644
--- a/drivers/soc/renesas/Kconfig
+++ b/drivers/soc/renesas/Kconfig
@@ -14,6 +14,7 @@ config SOC_RENESAS
select SYSC_R8A7794 if ARCH_R8A7794
select SYSC_R8A7795 if ARCH_R8A7795
select SYSC_R8A7796 if ARCH_R8A7796
+   select SYSC_R8A77965 if ARCH_R8A77965
select SYSC_R8A77970 if ARCH_R8A77970
select SYSC_R8A77980 if ARCH_R8A77980
select SYSC_R8A77995 if ARCH_R8A77995
@@ -57,6 +58,10 @@ config SYSC_R8A7796
bool "R-Car M3-W System Controller support" if COMPILE_TEST
select SYSC_RCAR
 
+config SYSC_R8A77965
+   bool "R-Car M3-N System Controller support" if COMPILE_TEST
+   select SYSC_RCAR
+
 config SYSC_R8A77970
bool "R-Car V3M System Controller support" if COMPILE_TEST
select SYSC_RCAR
diff --git a/drivers/soc/renesas/Makefile b/drivers/soc/renesas/Makefile
index d3b7bb3284c0..ccb5ec57a262 100644
--- a/drivers/soc/renesas/Makefile
+++ b/drivers/soc/renesas/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_SYSC_R8A7792)+= r8a7792-sysc.o
 obj-$(CONFIG_SYSC_R8A7794) += r8a7794-sysc.o
 obj-$(CONFIG_SYSC_R8A7795) += r8a7795-sysc.o
 obj-$(CONFIG_SYSC_R8A7796) += r8a7796-sysc.o
+obj-$(CONFIG_SYSC_R8A77965)+= r8a77965-sysc.o
 obj-$(CONFIG_SYSC_R8A77970)+= r8a77970-sysc.o
 obj-$(CONFIG_SYSC_R8A77980)+= r8a77980-sysc.o
 obj-$(CONFIG_SYSC_R8A77995)+= r8a77995-sysc.o
diff --git a/drivers/soc/renesas/r8a77965-sysc.c 
b/drivers/soc/renesas/r8a77965-sysc.c
new file mode 100644
index ..d7f7928e3c07
--- /dev/null
+++ b/drivers/soc/renesas/r8a77965-sysc.c
@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Renesas R-Car M3-N System Controller
+ * Copyright (C) 2018 Jacopo Mondi 
+ *
+ * Based on Renesas R-Car M3-W System Controller
+ * Copyright (C) 2016 Glider bvba
+ */
+
+#include 
+#include 
+
+#include 
+
+#include "rcar-sysc.h"
+
+static const struct rcar_sysc_area r8a77965_areas[] __initconst = {
+   { "always-on",  0, 0, R8A77965_PD_ALWAYS_ON, -1, PD_ALWAYS_ON },
+   { "ca57-scu",   0x1c0, 0, R8A77965_PD_CA57_SCU, R8A77965_PD_ALWAYS_ON,
+ PD_SCU },
+   { "ca57-cpu0",   0x80, 0, R8A77965_PD_CA57_CPU0, R8A77965_PD_CA57_SCU,
+ PD_CPU_NOCR },
+   { "ca57-cpu1",   0x80, 1, R8A77965_PD_CA57_CPU1, R8A77965_PD_CA57_SCU,
+ PD_CPU_NOCR },
+   { "cr7",

Re: [PATCH v2 03/19] soc: renesas: Identify R-Car M3-N

[Simon Horman]

On Tue, Feb 20, 2018 at 04:40:34PM +0100, Geert Uytterhoeven wrote:
> Hi Jacopo,
> 
> On Tue, Feb 20, 2018 at 4:12 PM, Jacopo Mondi  
> wrote:
> > Add support for indentifying R-Car M3-N (R8A77965) SoC.
> >
> > Signed-off-by: Jacopo Mondi 
> 
> Reviewed-by: Geert Uytterhoeven 
> 
> > --- a/drivers/soc/renesas/renesas-soc.c
> > +++ b/drivers/soc/renesas/renesas-soc.c
> > @@ -149,6 +149,11 @@ static const struct renesas_soc soc_rcar_v3m 
> > __initconst __maybe_unused = {
> > .id = 0x54,
> >  };
> >
> > +static const struct renesas_soc soc_rcar_m3_n __initconst __maybe_unused = 
> > {
> > +   .family = _rcar_gen3,
> > +   .id = 0x55,
> > +};
> 
> Note that the list was sorted by SoC part number.

Thanks, I have moved the above structure to between
the similar structure for m3_w and v3m. The result is below.

> > +
> >  static const struct renesas_soc soc_rcar_v3h __initconst __maybe_unused = {
> > .family = _rcar_gen3,
> > .id = 0x56,
> > @@ -214,6 +219,9 @@ static const struct of_device_id renesas_socs[] 
> > __initconst = {
> >  #ifdef CONFIG_ARCH_R8A7796
> > { .compatible = "renesas,r8a7796",  .data = _rcar_m3_w },
> >  #endif
> > +#ifdef CONFIG_ARCH_R8A77965
> > +   { .compatible = "renesas,r8a77965", .data = _rcar_m3_n },
> > +#endif
> >  #ifdef CONFIG_ARCH_R8A77970
> > { .compatible = "renesas,r8a77970", .data = _rcar_v3m },
> >  #endif

From: Jacopo Mondi 
Subject: [PATCH] soc: renesas: Identify R-Car M3-N

Add support for indentifying R-Car M3-N (R8A77965) SoC.

Signed-off-by: Jacopo Mondi 
Reviewed-by: Geert Uytterhoeven 
Signed-off-by: Simon Horman 
---
 drivers/soc/renesas/renesas-soc.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/soc/renesas/renesas-soc.c 
b/drivers/soc/renesas/renesas-soc.c
index 000834321774..ea71c413c926 100644
--- a/drivers/soc/renesas/renesas-soc.c
+++ b/drivers/soc/renesas/renesas-soc.c
@@ -144,6 +144,11 @@ static const struct renesas_soc soc_rcar_m3_w __initconst 
__maybe_unused = {
.id = 0x52,
 };
 
+static const struct renesas_soc soc_rcar_m3_n __initconst __maybe_unused = {
+   .family = _rcar_gen3,
+   .id = 0x55,
+};
+
 static const struct renesas_soc soc_rcar_v3m __initconst __maybe_unused = {
.family = _rcar_gen3,
.id = 0x54,
@@ -214,6 +219,9 @@ static const struct of_device_id renesas_socs[] __initconst 
= {
 #ifdef CONFIG_ARCH_R8A7796
{ .compatible = "renesas,r8a7796",  .data = _rcar_m3_w },
 #endif
+#ifdef CONFIG_ARCH_R8A77965
+   { .compatible = "renesas,r8a77965", .data = _rcar_m3_n },
+#endif
 #ifdef CONFIG_ARCH_R8A77970
{ .compatible = "renesas,r8a77970", .data = _rcar_v3m },
 #endif
-- 
2.11.0

Re: [PATCH 2/2] vfio: platform: Add generic DT reset support

[Philipp Zabel]

Hi Geert,

I have a suggestion to avoid having to use the IS_ERR_OR_NULL macro, see
below:

On Tue, 2018-02-13 at 17:36 +0100, Geert Uytterhoeven wrote:
> Vfio-platform requires reset support, provided either by ACPI, or, on DT
> platforms, by a device-specific reset driver matching against the
> device's compatible value.
> 
> On many SoCs, devices are connected to an SoC-internal reset controller,
> and can be reset in a generic way.  Hence add support to reset such
> devices using the reset controller subsystem, provided the reset
> hierarchy is described correctly in DT using the "resets" property.
> 
> Devices that require a more complex reset procedure can still
> provide a device-specific reset driver, as that takes precedence.
> 
> Note that this functionality depends on CONFIG_RESET_CONTROLLER=y, and
> becomes a no-op if reset controller support is disabled.
> 
> Signed-off-by: Geert Uytterhoeven 
> ---
>  drivers/vfio/platform/vfio_platform_common.c  | 23 +--
>  drivers/vfio/platform/vfio_platform_private.h |  1 +
>  2 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/vfio/platform/vfio_platform_common.c 
> b/drivers/vfio/platform/vfio_platform_common.c
> index b60bb5326668498c..5d1e48f96e423508 100644
> --- a/drivers/vfio/platform/vfio_platform_common.c
> +++ b/drivers/vfio/platform/vfio_platform_common.c
> @@ -17,6 +17,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -112,7 +113,13 @@ static bool vfio_platform_has_reset(struct 
> vfio_platform_device *vdev)
>   if (VFIO_PLATFORM_IS_ACPI(vdev))
>   return vfio_platform_acpi_has_reset(vdev);
>  
> - return vdev->of_reset ? true : false;
> + if (vdev->of_reset)
> + return true;
> +
> + if (!IS_ERR_OR_NULL(vdev->reset_control))
> + return true;

I'd avoid storing error values in vdev->reset_control at all, so this
could be:

if (vdev->reset_control)
return true;

> +
> + return false;
>  }
>  
>  static int vfio_platform_get_reset(struct vfio_platform_device *vdev)
> @@ -127,8 +134,15 @@ static int vfio_platform_get_reset(struct 
> vfio_platform_device *vdev)
>   vdev->of_reset = vfio_platform_lookup_reset(vdev->compat,
>   >reset_module);
>   }
> + if (vdev->of_reset)
> + return 0;
> +
> + vdev->reset_control = __of_reset_control_get(vdev->device->of_node,
> +  NULL, 0, false, false);
> + if (!IS_ERR(vdev->reset_control))
> + return 0;

if you assign to a local variable first here:

struct reset_control *rstc;

 ...

rstc = of_reset_control_get_exclusive(vdev->device->of_node, NULL);
if (!IS_ERR(rstc)) {
vdev->reset_control = rstc;
return 0;
}

Also, please don't use __of_reset_control_get directly.

>  
> - return vdev->of_reset ? 0 : -ENOENT;
> + return PTR_ERR(vdev->reset_control);

return PTR_ERR(rstc);

>  }
>  
>  static void vfio_platform_put_reset(struct vfio_platform_device *vdev)
> @@ -138,6 +152,8 @@ static void vfio_platform_put_reset(struct 
> vfio_platform_device *vdev)
>  
>   if (vdev->of_reset)
>   module_put(vdev->reset_module);
> +
> + reset_control_put(vdev->reset_control);
>  }
>  
>  static int vfio_platform_regions_init(struct vfio_platform_device *vdev)
> @@ -217,6 +233,9 @@ static int vfio_platform_call_reset(struct 
> vfio_platform_device *vdev,
>   } else if (vdev->of_reset) {
>   dev_info(vdev->device, "reset\n");
>   return vdev->of_reset(vdev);
> + } else if (!IS_ERR_OR_NULL(vdev->reset_control)) {
> + dev_info(vdev->device, "reset\n");
> + return reset_control_reset(vdev->reset_control);

} else {
if (vdev->reset_control)
dev_info(vdev->device, "reset\n");
return reset_control_reset(vdev->reset_control);

>   }
>  
>   dev_warn(vdev->device, "no reset function found!\n");
> diff --git a/drivers/vfio/platform/vfio_platform_private.h 
> b/drivers/vfio/platform/vfio_platform_private.h
> index 85ffe5d9d1abd94e..a56e80ae5986540b 100644
> --- a/drivers/vfio/platform/vfio_platform_private.h
> +++ b/drivers/vfio/platform/vfio_platform_private.h
> @@ -60,6 +60,7 @@ struct vfio_platform_device {
>   const char  *compat;
>   const char  *acpihid;
>   struct module   *reset_module;
> + struct reset_control*reset_control;
>   struct device   *device;
>  
>   /*

regards
Philipp

Re: [PATCH v2 02/19] soc: renesas: rcar-rst: Add support for R-Car M3-N

[Simon Horman]

On Tue, Feb 20, 2018 at 04:29:37PM +0100, Geert Uytterhoeven wrote:
> On Tue, Feb 20, 2018 at 4:12 PM, Jacopo Mondi  
> wrote:
> > Signed-off-by: Jacopo Mondi 
> 
> Reviewed-by: Geert Uytterhoeven 

Thanks, applied.

Re: [PATCH v9 03/11] media: platform: Add Renesas CEU driver

[jacopo mondi]

Hi Laurent, Hans,

On Wed, Feb 21, 2018 at 02:02:59PM +0100, Hans Verkuil wrote:
> On 02/21/18 13:29, Laurent Pinchart wrote:
> > Hi Hans,
> >
> > On Wednesday, 21 February 2018 14:03:24 EET Hans Verkuil wrote:
> >> On 02/19/18 17:59, 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 | 1661 +
> >>>  3 files changed, 1671 insertions(+)
> >>>  create mode 100644 drivers/media/platform/renesas-ceu.c
> >>
> >> 
> >>
> >>> +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;
> >>> +
> >>> + if (i >= ceudev->num_sd)
> >>> + return -EINVAL;
> >>> +
> >>> + if (vb2_is_streaming(>vb2_vq))
> >>> + return -EBUSY;
> >>> +
> >>> + if (i == ceudev->sd_index)
> >>> + return 0;
> >>> +
> >>> + ceu_sd_old = ceudev->sd;
> >>> + ceudev->sd = >subdevs[i];
> >>> +
> >>> + /* Make sure we can generate output image formats. */
> >>> + ret = ceu_init_formats(ceudev);
> >>
> >> Why is this done for every s_input? I would expect that this is done only
> >> once for each subdev.
> >>
> >> I also expect to see a ceu_set_default_fmt() call here. Or that the 
> >> v4l2_pix
> >> is kept in ceu_subdev (i.e. per subdev) instead of a single fmt in cuedev.
> >> I think I prefer that over configuring a new default format every time you
> >> switch inputs.
> >
> > What does userspace expect today ? I don't think we document anywhere that
> > formats are stored per-input in drivers. The VIDIOC_S_INPUT documentation is
> > very vague:
> >
> > "To select a video input applications store the number of the desired input 
> > in
> > an integer and call the VIDIOC_S_INPUT ioctl with a pointer to this integer.
> > Side effects are possible. For example inputs may support different video
> > standards, so the driver may implicitly switch the current standard. Because
> > of these possible side effects applications must select an input before
> > querying or negotiating any other parameters."
> >
> > I understand that as meaning "anything can happen when you switch inputs, so
> > be prepared to reconfigure everything explicitly without assuming any
> > particular parameter to have a sane value".
> >
> >> This code will work for two subdevs with exactly the same
> >> formats/properties. But switching between e.g. a sensor and a video
> >> receiver will leave things in an inconsistent state as far as I can see.
> >>
> >> E.g. if input 1 is the video receiver then switching to that input and
> >> running 'v4l2-ctl -V' will show the sensor format, not the video receiver
> >> format.
> >
> > I agree that the format should be consistent with the selected input, as
> > calling VIDIOC_S_INPUT immediately followed by a stream start sequence 
> > without
> > configuring formats should work (but produce a format that is not known to
> > userspace). My question is whether we should reset to a default format for 
> > the
> > newly select input, or switch back to the previously set format. I'd tend to
> > go for the former to keep as little state as possible, but I'm open to
> > counter-proposals.
>
> What to do is up to the driver. My personal preference is to make it 
> persistent
> per input, but that's just me. I won't block the other approach (resetting it
> to a default). Be aware that for video receivers the format depends on the 
> current
> selected standard as well. I think the code does that correctly already, but 
> it
> would be good to verify if possible.
>
> BTW, I think it is right that the spec isn't specific about what changes when
> you switch inputs. It can be quite complex for drivers to handle this and it
> is not unreasonable in my view for userspace to just assume it needs to 
> configure
> from scratch after switching inputs.
>

Given that there are not strict requisites here I will go for the
default format selection.

While there I'll rename the ceu_init_formats() function to ceu_init_mbus_fmt() 
as
that's what it actually does, and move ceudev->field initialization
from there  to ceu_set[_default]_fmt() functions.

I'm sending v10 with this changes hopefully quite soon.

Thanks
   j

> Regards,
>
>   Hans
>
> >
> >>> + if (ret) {
> >>> + ceudev->sd = 

Re: [PATCH v4 00/16] R-Car DU: Convert LVDS code to bridge driver

[Simon Horman]

On Wed, Feb 21, 2018 at 01:10:30AM +0200, Laurent Pinchart wrote:
> Hello,
> 
> This patch series addresses a design mistake that dates back from the initial
> DU support. Support for the LVDS encoders, which are IP cores separate from
> the DU, was bundled in the DU driver. Worse, both the DU and LVDS were
> described through a single DT node.
> 
> To fix the, patches 01/16 and 02/16 define new DT bindings for the LVDS
> encoders, and deprecate their description inside the DU bindings. To retain
> backward compatibility with existing DT, patches 03/16 to 08/16 then patch the
> device tree at runtime to convert the legacy bindings to the new ones.
> 
> With the DT side addressed, patch 09/16 converts the LVDS support code to a
> separate bridge driver. Patches 11/16 to 16/16 then update all the device tree
> sources to the new DU and LVDS encoders bindings.
> 
> I decided to go for live DT patching in patch 08/16 because implementing
> support for both the legacy and new bindings in the driver would have been
> very intrusive, and prevented further cleanups. This version relies more
> heavily on overlays to avoid touching the internals of the OF core compared to
> v2, even if manual fixes to the device tree are still needed.
> 
> Compared to v3, this series uses the OF changeset API to update properties
> instead of accessing the internals of the property structure. This removes the
> local implementation of functions to look up nodes by path and update
> properties. In order to do this, I pulled in Pantelis' patch series titled
> "[PATCH v2 0/5] of: dynamic: Changesets helpers & fixes" at Rob's request, and
> rebased it while taking two small review comments into account.
> 
> Rob, I'd like this series to be merged in v4.17. As the changeset helpers are
> now a dependency, I'd need you to merge them early (ideally on top of
> v4.16-rc1) and provide a stable branch, or get your ack to merge them through
> Dave's tree if they don't conflict with what you have and will queue for
> v4.17.
> 
> This version also drops the small fix to the Porter board device tree that has
> been queued for v4.17 already.
> 
> Compared to v2, the biggest change is in patch 03/16. Following Rob's and
> Frank's reviews it was clear that modifying the unflattened DT structure of
> the overlay before applying it wasn't popular. I have thus decided to use one
> overlay source per SoC to move as much of the DT changes to the overlay as
> possible, and only perform manual modifications (that are still needed as some
> of the information is board-specific) on the system DT after applying the
> overlay. As a result the overlay is parsed and applied without being modified.
> 
> Compared to v1, this series update the r8a7792 and r8a7794 device tree sources
> and incorporate review feedback as described by the changelogs of individual
> patches.
> 
> 
> Laurent Pinchart (11):
>   dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings
>   dt-bindings: display: renesas: Deprecate LVDS support in the DU
> bindings
>   drm: rcar-du: Fix legacy DT to create LVDS encoder nodes
>   drm: rcar-du: Convert LVDS encoder code to bridge driver
>   ARM: dts: r8a7790: Convert to new LVDS DT bindings
>   ARM: dts: r8a7791: Convert to new LVDS DT bindings
>   ARM: dts: r8a7792: Convert to new DU DT bindings
>   ARM: dts: r8a7793: Convert to new LVDS DT bindings
>   ARM: dts: r8a7794: Convert to new DU DT bindings
>   arm64: dts: renesas: r8a7795: Convert to new LVDS DT bindings
>   arm64: dts: renesas: r8a7796: Convert to new LVDS DT bindings

I have marked the dts patches above as deferred as they depend
on the driver changes not to cause a regression. Please repost them
or otherwise ping me once the driver dependencies are present in an rc
release.

I am assuming that the other patches in this series are not targeted
at the renesas tree.

> 
> Pantelis Antoniou (5):
>   of: dynamic: Add __of_node_dupv()
>   of: changesets: Introduce changeset helper methods
>   of: changeset: Add of_changeset_node_move method
>   of: unittest: changeset helpers
>   i2c: demux: Use changeset helpers for clarity

...

Re: [PATCH v5 00/26] Fix watchdog on Renesas R-Car Gen2 and RZ/G1

[Geert Uytterhoeven]

Hi Simon,

On Wed, Feb 21, 2018 at 5:13 PM, Simon Horman  wrote:
> On Tue, Feb 20, 2018 at 01:51:28PM +0100, Geert Uytterhoeven wrote:
>> On Mon, Feb 12, 2018 at 6:44 PM, Fabrizio Castro
>>  wrote:
>> > this series has been around for some time as RFC, and it has collected
>> > useful comments from the community along the way.
>> > The solution proposed by this patch set works for most R-Car Gen2 and
>> > RZ/G1 devices, but not all of them. We now know that for some R-Car
>> > Gen2 early revisions there is no proper software fix. Anyway, no
>> > product has been built around early revisions, but development boards
>> > mounting early revisions (basically prototypes) are still out there.
>> > As a result, this series isn't enabling the internal watchdog on R-Car
>> > Gen2 boards, developers may enable it in board specific device trees
>> > if needed.
>> > This series has been tested by me on the iwg20d, iwg22d, Lager, Alt,
>> > and Koelsch boards.
>> >
>> > The problem
>> > ===
>> > To deal with SMP on R-Car Gen2 and RZ/G1, we install a reset vector
>> > to ICRAM1 and we program the [S]BAR registers so that when we turn ON
>> > the non-boot CPUs they are redirected to the reset vector installed by
>> > Linux in ICRAM1, and eventually they continue the execution to RAM,
>> > where the SMP bring-up code will take care of the rest.
>> > The content of the [S]BAR registers survives a watchdog triggered reset,
>> > and as such after the watchdog fires the boot core will try and execute
>> > the SMP bring-up code instead of jumping to the bootrom code.
>> >
>> > The fix
>> > ===
>> > The main strategy for the solution is to let the reset vector decide
>> > if it needs to jump to shmobile_boot_fn or to the bootrom code.
>> > In a watchdog triggered reset scenario, since the [S]BAR registers keep
>> > their values, the boot CPU will jump into the newly designed reset
>> > vector, the assembly routine will eventually test WOVF (a bit in register
>> > RWTCSRA that indicates if the watchdog counter has overflown, the value
>> > of this bit gets retained in this scenario), and jump to the bootrom code
>> > which will in turn load up the bootloader, etc.
>> > When bringing up SMP or using CPU hotplug, the reset vector will jump
>> > to shmobile_boot_fn instead.
>> >
>> > Thank you All for your help.
>> >
>> > Best regards,
>> >
>> > Fabrizio Castro (26):
>> >   ARM: shmobile: Add watchdog support
>> >   ARM: dts: r8a7743: Adjust SMP routine size
>> >   ARM: dts: r8a7745: Adjust SMP routine size
>> >   ARM: dts: r8a7790: Adjust SMP routine size
>> >   ARM: dts: r8a7791: Adjust SMP routine size
>> >   ARM: dts: r8a7792: Adjust SMP routine size
>> >   ARM: dts: r8a7793: Adjust SMP routine size
>> >   ARM: dts: r8a7794: Adjust SMP routine size
>> >   soc: renesas: rcar-rst: Enable watchdog as reset trigger for Gen2
>> >   ARM: shmobile: rcar-gen2: Add watchdog support
>> >   dt-bindings: watchdog: renesas-wdt: Add R-Car Gen2 support
>> >   watchdog: renesas_wdt: Add R-Car Gen2 support
>> >   watchdog: renesas_wdt: Add restart handler
>> >   ARM: shmobile: defconfig: Enable RENESAS_WDT_GEN
>> >   clk: renesas: r8a7743: Add rwdt clock
>> >   clk: renesas: r8a7745: Add rwdt clock
>> >   clk: renesas: r8a7790: Add rwdt clock
>> >   clk: renesas: r8a7791/r8a7793: Add rwdt clock
>> >   clk: renesas: r8a7794: Add rwdt clock
>> >   ARM: dts: r8a7743: Add watchdog support to SoC dtsi
>> >   ARM: dts: r8a7745: Add watchdog support to SoC dtsi
>> >   ARM: dts: r8a7790: Add watchdog support to SoC dtsi
>> >   ARM: dts: r8a7791: Add watchdog support to SoC dtsi
>> >   ARM: dts: r8a7794: Add watchdog support to SoC dtsi
>> >   ARM: dts: iwg20m: Add watchdog support to SoM dtsi
>> >   ARM: dts: iwg22m: Add watchdog support to SoM dtsi
>> >
>> >  .../devicetree/bindings/watchdog/renesas-wdt.txt   | 19 ++--
>> >  arch/arm/boot/dts/r8a7743-iwg20m.dtsi  |  5 ++
>> >  arch/arm/boot/dts/r8a7743.dtsi | 12 -
>> >  arch/arm/boot/dts/r8a7745-iwg22m.dtsi  |  5 ++
>> >  arch/arm/boot/dts/r8a7745.dtsi | 12 -
>> >  arch/arm/boot/dts/r8a7790.dtsi | 12 -
>> >  arch/arm/boot/dts/r8a7791.dtsi | 12 -
>> >  arch/arm/boot/dts/r8a7792.dtsi |  2 +-
>> >  arch/arm/boot/dts/r8a7793.dtsi |  2 +-
>> >  arch/arm/boot/dts/r8a7794.dtsi | 12 -
>> >  arch/arm/configs/shmobile_defconfig|  1 +
>> >  arch/arm/mach-shmobile/common.h|  6 +++
>> >  arch/arm/mach-shmobile/headsmp.S   | 55 
>> > ++
>> >  arch/arm/mach-shmobile/platsmp-apmu.c  |  1 +
>> >  arch/arm/mach-shmobile/pm-rcar-gen2.c  | 15 --
>> >  drivers/clk/renesas/r8a7743-cpg-mssr.c |  2 +
>> >  drivers/clk/renesas/r8a7745-cpg-mssr.c |  2 +
>> > 

Re: [PATCH v9 11/11] media: i2c: ov7670: Fully set mbus frame fmt

[Hans Verkuil]

On 02/21/2018 04:47 PM, jacopo mondi wrote:
> Hello again,
> 
> On Tue, Feb 20, 2018 at 09:58:57AM +0100, jacopo mondi wrote:
>> Hi Laurent,
>>
>> On Mon, Feb 19, 2018 at 09:19:32PM +0200, Laurent Pinchart wrote:
>>> Hi Jacopo,
>>>
>>> Thank you for the patch.
>>>
>>> On Monday, 19 February 2018 18:59:44 EET Jacopo Mondi wrote:
 The sensor driver sets mbus format colorspace information and sizes,
 but not ycbcr encoding, quantization and xfer function. When supplied
 with an badly initialized mbus frame format structure, those fields
 need to be set explicitly not to leave them uninitialized. This is
 tested by v4l2-compliance, which supplies a mbus format description
 structure and checks for all fields to be properly set.

 Without this commit, v4l2-compliance fails when testing formats with:
 fail: v4l2-test-formats.cpp(335): ycbcr_enc >= 0xff

 Signed-off-by: Jacopo Mondi 
 ---
  drivers/media/i2c/ov7670.c | 4 
  1 file changed, 4 insertions(+)

 diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
 index 25b26d4..61c472e 100644
 --- a/drivers/media/i2c/ov7670.c
 +++ b/drivers/media/i2c/ov7670.c
 @@ -996,6 +996,10 @@ static int ov7670_try_fmt_internal(struct v4l2_subdev
 *sd, fmt->height = wsize->height;
fmt->colorspace = ov7670_formats[index].colorspace;
>>>
>>> On a side note, if I'm not mistaken the colorspace field is set to SRGB for
>>> all entries. Shouldn't you hardcode it here and remove the field ?
>>>
 +  fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
 +  fmt->quantization = V4L2_QUANTIZATION_DEFAULT;
 +  fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT;
>>>
>>> How about setting the values explicitly instead of relying on defaults ? 
>>> That
>>> would be V4L2_YCBCR_ENC_601, V4L2_QUANTIZATION_LIM_RANGE and
>>> V4L2_XFER_FUNC_SRGB. And could you then check a captured frame to see if the
>>> sensor outputs limited or full range ?
>>>
>>
>> This actually makes me wonder if those informations (ycbcb_enc,
>> quantization and xfer_func) shouldn't actually be part of the
>> supported format list... I blindly added those default fields in the
>> try_fmt function, but I doubt they applies to all supported formats.
>>
>> Eg. the sensor supports YUYV as well as 2 RGB encodings (RGB444 and
>> RGB 565) and 1 raw format (BGGR). I now have a question here:
>>
>> 1) ycbcr_enc transforms non-linear R'G'B' to Y'CbCr: does this
>> applies to RGB and raw formats? I don't think so, and what value is
>> the correct one for the ycbcr_enc field in this case? I assume
>> xfer_func and quantization applies to all formats instead..
>>
> 
> What if I repropose this as separate patch not part of the CEU series
> in order not to hold back v10 (which I hope will be the last CEU
> iteration)?

I would definitely be fine with that. We first need to define what
exactly should be done by drivers. See also this thread:

https://www.spinics.net/lists/linux-renesas-soc/msg23888.html

I'll work on that going forward as part of the compliance tests.

Regards,

Hans

> 
>> Thanks
>>j
>>
info->format = *fmt;

return 0;
>>>
>>> --
>>> Regards,
>>>
>>> Laurent Pinchart
>>>

Re: [PATCH] watchdog: renesas_wdt: Blacklist early R-Car Gen2 SoCs

[Simon Horman]

On Wed, Feb 21, 2018 at 04:43:04PM +0100, Geert Uytterhoeven wrote:
> On early revisions of some R-Car Gen2 SoCs, and depending on SMP
> configuration, the system may fail to restart on watchdog time-out, and
> lock up instead.
> 
> Specifically:
>   - On R-Car H2 ES1.0 and M2-W ES1.0, watchdog restart fails unless
> only the first CPU core is in use (using e.g. the "maxcpus=1" kernel
> commandline option).
>   - On R-Car V2H ES1.1, watchdog restart fails unless SMP is disabled
> completely (using CONFIG_SMP=n during build configuration, or using
> the "nosmp" or "maxcpus=0" kernel commandline options).
> 
> Prevent using the watchdog in impacted cases by blacklisting the
> affected SoCs, using the minimum known working revisions (ES2.0 on R-Car
> H2, and ES3.0 on M2-W), and taking the actual SMP software configuration
> into account.
> 
> Signed-off-by: Geert Uytterhoeven 
> ---
> To be folded into Fabrizio Castro's "watchdog: renesas_wdt: Add R-Car
> Gen2 support".
> 
> Note that I cannot use IS_ENABLED(CONFIG_SMP), as setup_max_cpus does
> not exist for CONFIG_SMP=n.

Thanks, I was going to ask about that.

Reviewed-by: Simon Horman 


> Any reports on R-Car M2-W ES2.0 and V2H ES2.0+ are welcomed!
> 
> As the failure is due to an integration issue, and the watchdog itself
> is working fine, an alternative solution would be to move the check to
> the code that installs the reset trigger ("soc: renesas: rcar-rst:
> Enable watchdog as reset trigger for Gen2").
> However, doing so would mean that:
>   1. The user could enable and seemingly use the watchdog, but watchdog
>  timeout would not restart the system,
>   2. The same check should be done before installing the new reset
>  vector ("ARM: shmobile: rcar-gen2: Add watchdog support"), too,
>  else onlining CPU0 would fail once the watchdog has timed out.
> ---
>  drivers/watchdog/renesas_wdt.c | 43 
> ++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/drivers/watchdog/renesas_wdt.c b/drivers/watchdog/renesas_wdt.c
> index 0f88614797c36022..4c8e8d2600a922a5 100644
> --- a/drivers/watchdog/renesas_wdt.c
> +++ b/drivers/watchdog/renesas_wdt.c
> @@ -16,6 +16,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  
>  #define RWTCNT   0
> @@ -131,6 +133,44 @@ static const struct watchdog_ops rwdt_ops = {
>   .restart = rwdt_restart,
>  };
>  
> +#if defined(CONFIG_ARCH_RCAR_GEN2) && defined(CONFIG_SMP)
> +/*
> + * Watchdog-reset integration is broken on early revisions of R-Car Gen2 SoCs
> + */
> +static const struct soc_device_attribute rwdt_quirks_match[] = {
> + {
> + .soc_id = "r8a7790",
> + .revision = "ES1.*",
> + .data = (void *)1,  /* needs single CPU */
> + }, {
> + .soc_id = "r8a7791",
> + .revision = "ES[12].*",
> + .data = (void *)1,  /* needs single CPU */
> + }, {
> + .soc_id = "r8a7792",
> + .revision = "*",
> + .data = (void *)0,  /* needs SMP disabled */
> + },
> + { /* sentinel */ }
> +};
> +
> +static bool rwdt_blacklisted(struct device *dev)
> +{
> + const struct soc_device_attribute *attr;
> +
> + attr = soc_device_match(rwdt_quirks_match);
> + if (attr && setup_max_cpus > (uintptr_t)attr->data) {
> + dev_info(dev, "Watchdog blacklisted on %s %s\n", attr->soc_id,
> +  attr->revision);
> + return true;
> + }
> +
> + return false;
> +}
> +#else /* !CONFIG_ARCH_RCAR_GEN2 || !CONFIG_SMP */
> +static inline bool rwdt_blacklisted(struct device *dev) { return false; }
> +#endif /* !CONFIG_ARCH_RCAR_GEN2 || !CONFIG_SMP */
> +
>  static int rwdt_probe(struct platform_device *pdev)
>  {
>   struct rwdt_priv *priv;
> @@ -139,6 +179,9 @@ static int rwdt_probe(struct platform_device *pdev)
>   unsigned long clks_per_sec;
>   int ret, i;
>  
> + if (rwdt_blacklisted(>dev))
> + return -ENODEV;
> +
>   priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL);
>   if (!priv)
>   return -ENOMEM;
> -- 
> 2.7.4
> 

Re: [PATCH v2] arm64: defconfig: Enable PWM and USB for R-Car

[Simon Horman]

On Wed, Feb 21, 2018 at 02:57:16PM +0900, Yoshihiro Shimoda wrote:
> Enables PWM controller, USB-DMAC that is used by HS-USB, USB 3.0
> peripheral controller and USB 3.0 PHY for R-Car SoCs.
> 
> Signed-off-by: Yoshihiro Shimoda 
> Acked-by: Geert Uytterhoeven 

Thanks, applied.

Re: [PATCH v5 00/26] Fix watchdog on Renesas R-Car Gen2 and RZ/G1

[Simon Horman]

On Tue, Feb 20, 2018 at 01:51:28PM +0100, Geert Uytterhoeven wrote:
> On Mon, Feb 12, 2018 at 6:44 PM, Fabrizio Castro
>  wrote:
> > this series has been around for some time as RFC, and it has collected
> > useful comments from the community along the way.
> > The solution proposed by this patch set works for most R-Car Gen2 and
> > RZ/G1 devices, but not all of them. We now know that for some R-Car
> > Gen2 early revisions there is no proper software fix. Anyway, no
> > product has been built around early revisions, but development boards
> > mounting early revisions (basically prototypes) are still out there.
> > As a result, this series isn't enabling the internal watchdog on R-Car
> > Gen2 boards, developers may enable it in board specific device trees
> > if needed.
> > This series has been tested by me on the iwg20d, iwg22d, Lager, Alt,
> > and Koelsch boards.
> >
> > The problem
> > ===
> > To deal with SMP on R-Car Gen2 and RZ/G1, we install a reset vector
> > to ICRAM1 and we program the [S]BAR registers so that when we turn ON
> > the non-boot CPUs they are redirected to the reset vector installed by
> > Linux in ICRAM1, and eventually they continue the execution to RAM,
> > where the SMP bring-up code will take care of the rest.
> > The content of the [S]BAR registers survives a watchdog triggered reset,
> > and as such after the watchdog fires the boot core will try and execute
> > the SMP bring-up code instead of jumping to the bootrom code.
> >
> > The fix
> > ===
> > The main strategy for the solution is to let the reset vector decide
> > if it needs to jump to shmobile_boot_fn or to the bootrom code.
> > In a watchdog triggered reset scenario, since the [S]BAR registers keep
> > their values, the boot CPU will jump into the newly designed reset
> > vector, the assembly routine will eventually test WOVF (a bit in register
> > RWTCSRA that indicates if the watchdog counter has overflown, the value
> > of this bit gets retained in this scenario), and jump to the bootrom code
> > which will in turn load up the bootloader, etc.
> > When bringing up SMP or using CPU hotplug, the reset vector will jump
> > to shmobile_boot_fn instead.
> >
> > Thank you All for your help.
> >
> > Best regards,
> >
> > Fabrizio Castro (26):
> >   ARM: shmobile: Add watchdog support
> >   ARM: dts: r8a7743: Adjust SMP routine size
> >   ARM: dts: r8a7745: Adjust SMP routine size
> >   ARM: dts: r8a7790: Adjust SMP routine size
> >   ARM: dts: r8a7791: Adjust SMP routine size
> >   ARM: dts: r8a7792: Adjust SMP routine size
> >   ARM: dts: r8a7793: Adjust SMP routine size
> >   ARM: dts: r8a7794: Adjust SMP routine size
> >   soc: renesas: rcar-rst: Enable watchdog as reset trigger for Gen2
> >   ARM: shmobile: rcar-gen2: Add watchdog support
> >   dt-bindings: watchdog: renesas-wdt: Add R-Car Gen2 support
> >   watchdog: renesas_wdt: Add R-Car Gen2 support
> >   watchdog: renesas_wdt: Add restart handler
> >   ARM: shmobile: defconfig: Enable RENESAS_WDT_GEN
> >   clk: renesas: r8a7743: Add rwdt clock
> >   clk: renesas: r8a7745: Add rwdt clock
> >   clk: renesas: r8a7790: Add rwdt clock
> >   clk: renesas: r8a7791/r8a7793: Add rwdt clock
> >   clk: renesas: r8a7794: Add rwdt clock
> >   ARM: dts: r8a7743: Add watchdog support to SoC dtsi
> >   ARM: dts: r8a7745: Add watchdog support to SoC dtsi
> >   ARM: dts: r8a7790: Add watchdog support to SoC dtsi
> >   ARM: dts: r8a7791: Add watchdog support to SoC dtsi
> >   ARM: dts: r8a7794: Add watchdog support to SoC dtsi
> >   ARM: dts: iwg20m: Add watchdog support to SoM dtsi
> >   ARM: dts: iwg22m: Add watchdog support to SoM dtsi
> >
> >  .../devicetree/bindings/watchdog/renesas-wdt.txt   | 19 ++--
> >  arch/arm/boot/dts/r8a7743-iwg20m.dtsi  |  5 ++
> >  arch/arm/boot/dts/r8a7743.dtsi | 12 -
> >  arch/arm/boot/dts/r8a7745-iwg22m.dtsi  |  5 ++
> >  arch/arm/boot/dts/r8a7745.dtsi | 12 -
> >  arch/arm/boot/dts/r8a7790.dtsi | 12 -
> >  arch/arm/boot/dts/r8a7791.dtsi | 12 -
> >  arch/arm/boot/dts/r8a7792.dtsi |  2 +-
> >  arch/arm/boot/dts/r8a7793.dtsi |  2 +-
> >  arch/arm/boot/dts/r8a7794.dtsi | 12 -
> >  arch/arm/configs/shmobile_defconfig|  1 +
> >  arch/arm/mach-shmobile/common.h|  6 +++
> >  arch/arm/mach-shmobile/headsmp.S   | 55 
> > ++
> >  arch/arm/mach-shmobile/platsmp-apmu.c  |  1 +
> >  arch/arm/mach-shmobile/pm-rcar-gen2.c  | 15 --
> >  drivers/clk/renesas/r8a7743-cpg-mssr.c |  2 +
> >  drivers/clk/renesas/r8a7745-cpg-mssr.c |  2 +
> >  drivers/clk/renesas/r8a7790-cpg-mssr.c |  2 +
> >  drivers/clk/renesas/r8a7791-cpg-mssr.c |  2 +
> >  drivers/clk/renesas/r8a7794-cpg-mssr.c

Re: [PATCH 2/2] vfio: platform: Add generic DT reset support

[Geert Uytterhoeven]

Hi Eric,

On Wed, Feb 14, 2018 at 11:11 AM, Auger Eric  wrote:
> On 14/02/18 10:43, Geert Uytterhoeven wrote:
>> On Wed, Feb 14, 2018 at 10:09 AM, Auger Eric  wrote:
>>> On 13/02/18 17:36, Geert Uytterhoeven wrote:
 Vfio-platform requires reset support, provided either by ACPI, or, on DT
 platforms, by a device-specific reset driver matching against the
 device's compatible value.

 On many SoCs, devices are connected to an SoC-internal reset controller,
 and can be reset in a generic way.  Hence add support to reset such
 devices using the reset controller subsystem, provided the reset
 hierarchy is described correctly in DT using the "resets" property.
>>>
>>> I first acknowledge I am not familiar with what those reset controllers
>>> do in practice. My fear is that we may rely on generic SW pieces that
>>> may not be adapted to passthrough constraints. We must guarantee that
>>> any DMA access attempted by the devices are stopped and any interrupts
>>> gets stopped. Can we guarantee that the reset controller always induce
>>> that? Otherwise we may leave the door opened to badly reset assigned
>>> devices.
>>
>> An on-SoC reset controller is basically a block controlling signals to the
>> reset inputs of the individual on-SoC devices.
>> On Renesas ARM SoCs, this allows to do a full reset of the attached device.
>>
>> Of course the exact semantics depend on the actual SoC.
> that's the issue actually.
>> If e.g. DMA and interrupts are not stopped for a specific device on a
>> specific SoC, it still needs a device-specific reset driver, matching against
>> the appropriate compatible value, cfr. the quoted paragraph below.
> yes but by default we still accept the reset controller solution.
>>
>> You could add a whitelist for of_machine_is_compatible() or
>> of_device_is_compatible(), but that will grow large fast.
> Could be the kind of solution needed. At the moment the list of assigned
> platform devices is pretty reduced.
>
> Couldn't we imagine additional dt properties to emphasize the fact a
> platform device is passthrough friendly in terms of reset, either
> through a reset controller or exposing a single reg that need to be
> reset for full reset to be achieved, in accordance with assignment
> constraints. That way, the driver writer would somehow certify the
> device is eligible for passthrough. One of the issue today is that the
> vfio platform reset driver is not maintained by the native driver
> maintainer.

I'm not so fond of adding more DT properties for this. They can be abused
as well.

In general, if there's a "resets" DT property, it means the device can be
reset through the pointed-by reset controller. So that's the common case,
which I'd like to optimize/simplify for.

If more is needed, a separate (device-specific) vfio_reset handler needs
to be written, by the people that know the hardware.

> I think if people want to do platform passthrough, they need to devise
> their HW IPs so that this reset modality is simplified by exposing this
> kind of single reg and then dt description may expose this. Also if
> possible, the dt node must be as simple/generic as possible to avoid
> writing a huge dt node creation function on QEMU side and avoid
> dependencies on other nodes.

Yes. It all depends on sane hardware ;-)

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 1/2] vfio: platform: Fix reset module leak in error path

[Geert Uytterhoeven]

Hi Eric,

On Wed, Feb 14, 2018 at 10:32 AM, Geert Uytterhoeven
 wrote:
> On Wed, Feb 14, 2018 at 9:36 AM, Auger Eric  wrote:
>> If I am not wrong we also leak the reset_module if
>> vfio_platform_get_reset() fails to find the reset function (of_reset ==
>> NULL), in which case we should do the module_put() in
>> vfio_platform_get_reset().
>
> Correct. Will look into fixing it...

Upon second look, I don't think there's a leak in vfio_platform_get_reset().

If try_module_get() succeeded, there will always be a valid reset function
(unless someone registered a vfio_reset_handler with a NULL reset function).

Or do you mean the call to request_module()?
That one doesn't do a module_get(), it merely tries to load a module.
Hence there's no need to do a module_put() afterwards.

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 19/19] ARM64: dts: r8a77965: Add EtherAVB device node

[Sergei Shtylyov]

Hello!

On 02/21/2018 01:07 PM, jacopo mondi wrote:

>>> Populate the ethernet@e680 device node to enable Ethernet interface
>>> for R-Car M3-N (r8a77965) SoC.
>>>
>>> Signed-off-by: Jacopo Mondi 
>>> Reviewed-by: Geert Uytterhoeven 
>>>
>>> ---
>>> v1 -> v2:
>>> - Replace ALWAYS_ON power area identifier with numeric constant
>>> ---
>>>  arch/arm64/boot/dts/renesas/r8a77965.dtsi | 43 
>>> ++-
>>>  1 file changed, 42 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi 
>>> b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
>>> index 55f05f7..c249895 100644
>>> --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
>>> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
>>> @@ -520,7 +520,48 @@
>>> };
>>>
>>> avb: ethernet@e680 {
>>> -   /* placeholder */
>>> +   compatible = "renesas,etheravb-r8a77965",
>>> +"renesas,etheravb-rcar-gen3";
>>> +   reg = <0 0xe680 0 0x800>, <0 0xe6a0 0 0x1>;
>>> +   interrupts = ,
>>> +,
>>> +,
>>> +,
>>> +,
>>> +,
>>> +,
>>> +,
>>> +,
>>> +,
>>> +,
>>> +,
>>> +,
>>> +,
>>> +,
>>> +,
>>> +,
>>> +,
>>> +,
>>> +,
>>> +,
>>> +,
>>> +,
>>> +,
>>> +;
>>> +   interrupt-names = "ch0", "ch1", "ch2", "ch3",
>>> + "ch4", "ch5", "ch6", "ch7",
>>> + "ch8", "ch9", "ch10", "ch11",
>>> + "ch12", "ch13", "ch14", "ch15",
>>> + "ch16", "ch17", "ch18", "ch19",
>>> + "ch20", "ch21", "ch22", "ch23",
>>> + "ch24";
>>> +   clocks = < CPG_MOD 812>;
>>> +   power-domains = < 32>;
>>> +   resets = < 812>;
>>> +   phy-mode = "rgmii-txid";
>>
>>Why not just "rgmii"? TX delay is a board specific detail, no?
>>
> 
> I admit I took this one straight from r8a7796 dtsi.
> Would you like to me resend and change this?

   Yes, unless Simon would fix it while merging...

> Thanks
>j
> 
>>> +   #address-cells = <1>;
>>> +   #size-cells = <0>;
>>> +   status = "disabled";
>>> };
>>>
>>> csi20: csi2@fea8 {

MBR, Sergei

Re: [PATCH v9 11/11] media: i2c: ov7670: Fully set mbus frame fmt

[jacopo mondi]

Hello again,

On Tue, Feb 20, 2018 at 09:58:57AM +0100, jacopo mondi wrote:
> Hi Laurent,
>
> On Mon, Feb 19, 2018 at 09:19:32PM +0200, Laurent Pinchart wrote:
> > Hi Jacopo,
> >
> > Thank you for the patch.
> >
> > On Monday, 19 February 2018 18:59:44 EET Jacopo Mondi wrote:
> > > The sensor driver sets mbus format colorspace information and sizes,
> > > but not ycbcr encoding, quantization and xfer function. When supplied
> > > with an badly initialized mbus frame format structure, those fields
> > > need to be set explicitly not to leave them uninitialized. This is
> > > tested by v4l2-compliance, which supplies a mbus format description
> > > structure and checks for all fields to be properly set.
> > >
> > > Without this commit, v4l2-compliance fails when testing formats with:
> > > fail: v4l2-test-formats.cpp(335): ycbcr_enc >= 0xff
> > >
> > > Signed-off-by: Jacopo Mondi 
> > > ---
> > >  drivers/media/i2c/ov7670.c | 4 
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
> > > index 25b26d4..61c472e 100644
> > > --- a/drivers/media/i2c/ov7670.c
> > > +++ b/drivers/media/i2c/ov7670.c
> > > @@ -996,6 +996,10 @@ static int ov7670_try_fmt_internal(struct v4l2_subdev
> > > *sd, fmt->height = wsize->height;
> > >   fmt->colorspace = ov7670_formats[index].colorspace;
> >
> > On a side note, if I'm not mistaken the colorspace field is set to SRGB for
> > all entries. Shouldn't you hardcode it here and remove the field ?
> >
> > > + fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> > > + fmt->quantization = V4L2_QUANTIZATION_DEFAULT;
> > > + fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT;
> >
> > How about setting the values explicitly instead of relying on defaults ? 
> > That
> > would be V4L2_YCBCR_ENC_601, V4L2_QUANTIZATION_LIM_RANGE and
> > V4L2_XFER_FUNC_SRGB. And could you then check a captured frame to see if the
> > sensor outputs limited or full range ?
> >
>
> This actually makes me wonder if those informations (ycbcb_enc,
> quantization and xfer_func) shouldn't actually be part of the
> supported format list... I blindly added those default fields in the
> try_fmt function, but I doubt they applies to all supported formats.
>
> Eg. the sensor supports YUYV as well as 2 RGB encodings (RGB444 and
> RGB 565) and 1 raw format (BGGR). I now have a question here:
>
> 1) ycbcr_enc transforms non-linear R'G'B' to Y'CbCr: does this
> applies to RGB and raw formats? I don't think so, and what value is
> the correct one for the ycbcr_enc field in this case? I assume
> xfer_func and quantization applies to all formats instead..
>

What if I repropose this as separate patch not part of the CEU series
in order not to hold back v10 (which I hope will be the last CEU
iteration)?

> Thanks
>j
>
> > >   info->format = *fmt;
> > >
> > >   return 0;
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
> >

Re: [PATCH v4 04/16] of: changesets: Introduce changeset helper methods

[Geert Uytterhoeven]

Hi Rob,

On Wed, Feb 21, 2018 at 4:23 PM, Rob Herring  wrote:
> On Wed, Feb 21, 2018 at 4:21 AM, Geert Uytterhoeven
>  wrote:
>> You missed one fix I have in my topic/overlays branch
>> https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/commit/?h=topic/overlays=150f95b9dec77ce371c229f7ac4d6dd8620bef4a
>
> Are you planning to try to upstream all this? If not, I'll get Frank

Not really. But I do need a way to load DT overlays at runtime, for testing
hardware on expansion connectors.

> to keep changing the overlay API to make carrying it out of tree more
> painful. :)

He already did a very good job w.r.t. that in v4.15-rc1 ;^)

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 v4 04/16] of: changesets: Introduce changeset helper methods

[Rob Herring]

On Wed, Feb 21, 2018 at 4:21 AM, Geert Uytterhoeven
 wrote:
> Hi Laurent,
>
> On Wed, Feb 21, 2018 at 12:10 AM, Laurent Pinchart
>  wrote:
>> From: Pantelis Antoniou 
>>
>> Changesets are very powerful, but the lack of a helper API
>> makes using them cumbersome. Introduce a simple copy based
>> API that makes things considerably easier.
>>
>> To wit, adding a property using the raw API.
>>
>> struct property *prop;
>> prop = kzalloc(sizeof(*prop)), GFP_KERNEL);
>> prop->name = kstrdup("compatible");
>> prop->value = kstrdup("foo,bar");
>> prop->length = strlen(prop->value) + 1;
>> of_changeset_add_property(ocs, np, prop);
>>
>> while using the helper API
>>
>> of_changeset_add_property_string(ocs, np, "compatible",
>> "foo,bar");
>>
>> Signed-off-by: Pantelis Antoniou 
>> [Fixed memory leak in __of_changeset_add_update_property_copy()]
>> Signed-off-by: Laurent Pinchart 
>
> You missed one fix I have in my topic/overlays branch
> https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/commit/?h=topic/overlays=150f95b9dec77ce371c229f7ac4d6dd8620bef4a

Are you planning to try to upstream all this? If not, I'll get Frank
to keep changing the overlay API to make carrying it out of tree more
painful. :)

Rob

Re: [PATCH v9 07/11] media: i2c: ov772x: Support frame interval handling

[Hans Verkuil]

On 02/21/18 16:16, jacopo mondi wrote:
>>>  static const struct v4l2_subdev_pad_ops ov772x_subdev_pad_ops = {
>>> -   .enum_mbus_code = ov772x_enum_mbus_code,
>>> -   .get_selection  = ov772x_get_selection,
>>> -   .get_fmt= ov772x_get_fmt,
>>> -   .set_fmt= ov772x_set_fmt,
>>> +   .enum_frame_interval= ov772x_enum_frame_interval,
>>> +   .enum_mbus_code = ov772x_enum_mbus_code,
>>> +   .get_selection  = ov772x_get_selection,
>>> +   .get_fmt= ov772x_get_fmt,
>>> +   .set_fmt= ov772x_set_fmt,
>>
>> Shouldn't these last four ops be added in the previous patch?
>> They don't have anything to do with the frame interval support.
>>
> 
> If you look closely you'll notice I have just re-aligned them, since I
> was at there to add enum_frame_interval operation

Ah, sorry. I missed that. Never mind then :-)

Hans

> 
>> Anyway, after taking care of the memsets and these four ops you can add
>> my:
>>
>> Acked-by: Hans Verkuil 
> 
> Thanks
>j
> 

Re: [PATCH v9 07/11] media: i2c: ov772x: Support frame interval handling

[jacopo mondi]

Hi Hans,

On Wed, Feb 21, 2018 at 01:12:14PM +0100, Hans Verkuil wrote:

[snip]

> > +static int ov772x_g_frame_interval(struct v4l2_subdev *sd,
> > +  struct v4l2_subdev_frame_interval *ival)
> > +{
> > +   struct ov772x_priv *priv = to_ov772x(sd);
> > +   struct v4l2_fract *tpf = >interval;
> > +
> > +   memset(ival->reserved, 0, sizeof(ival->reserved));
>
> This memset...
>
> > +   tpf->numerator = 1;
> > +   tpf->denominator = priv->fps;
> > +
> > +   return 0;
> > +}
> > +
> > +static int ov772x_s_frame_interval(struct v4l2_subdev *sd,
> > +  struct v4l2_subdev_frame_interval *ival)
> > +{
> > +   struct ov772x_priv *priv = to_ov772x(sd);
> > +   struct v4l2_fract *tpf = >interval;
> > +
> > +   memset(ival->reserved, 0, sizeof(ival->reserved));
>
> ... and this memset can be dropped. The core code will memset this for you.
>
>

I see! Ok, I'll drop them in v10

> > +
> > +   return ov772x_set_frame_rate(priv, tpf, priv->cfmt, priv->win);
> > +}
> > +
> >  static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl)
> >  {
> > struct ov772x_priv *priv = container_of(ctrl->handler,
> > @@ -757,6 +905,7 @@ static int ov772x_set_params(struct ov772x_priv *priv,
> >  const struct ov772x_win_size *win)
> >  {
> > struct i2c_client *client = v4l2_get_subdevdata(>subdev);
> > +   struct v4l2_fract tpf;
> > int ret;
> > u8  val;
> >
> > @@ -885,6 +1034,13 @@ static int ov772x_set_params(struct ov772x_priv *priv,
> > if (ret < 0)
> > goto ov772x_set_fmt_error;
> >
> > +   /* COM4, CLKRC: Set pixel clock and framerate. */
> > +   tpf.numerator = 1;
> > +   tpf.denominator = priv->fps;
> > +   ret = ov772x_set_frame_rate(priv, , cfmt, win);
> > +   if (ret < 0)
> > +   goto ov772x_set_fmt_error;
> > +
> > /*
> >  * set COM8
> >  */
> > @@ -1043,6 +1199,24 @@ static const struct v4l2_subdev_core_ops 
> > ov772x_subdev_core_ops = {
> > .s_power= ov772x_s_power,
> >  };
> >
> > +static int ov772x_enum_frame_interval(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_frame_interval_enum 
> > *fie)
> > +{
> > +   if (fie->pad || fie->index >= OV772X_N_FRAME_INTERVALS)
> > +   return -EINVAL;
> > +
> > +   if (fie->width != VGA_WIDTH && fie->width != QVGA_WIDTH)
> > +   return -EINVAL;
> > +   if (fie->height != VGA_HEIGHT && fie->height != QVGA_HEIGHT)
> > +   return -EINVAL;
> > +
> > +   fie->interval.numerator = 1;
> > +   fie->interval.denominator = ov772x_frame_intervals[fie->index];
> > +
> > +   return 0;
> > +}
> > +
> >  static int ov772x_enum_mbus_code(struct v4l2_subdev *sd,
> > struct v4l2_subdev_pad_config *cfg,
> > struct v4l2_subdev_mbus_code_enum *code)
> > @@ -1055,14 +1229,17 @@ static int ov772x_enum_mbus_code(struct v4l2_subdev 
> > *sd,
> >  }
> >
> >  static const struct v4l2_subdev_video_ops ov772x_subdev_video_ops = {
> > -   .s_stream   = ov772x_s_stream,
> > +   .s_stream   = ov772x_s_stream,
> > +   .s_frame_interval   = ov772x_s_frame_interval,
> > +   .g_frame_interval   = ov772x_g_frame_interval,
> >  };
> >
> >  static const struct v4l2_subdev_pad_ops ov772x_subdev_pad_ops = {
> > -   .enum_mbus_code = ov772x_enum_mbus_code,
> > -   .get_selection  = ov772x_get_selection,
> > -   .get_fmt= ov772x_get_fmt,
> > -   .set_fmt= ov772x_set_fmt,
> > +   .enum_frame_interval= ov772x_enum_frame_interval,
> > +   .enum_mbus_code = ov772x_enum_mbus_code,
> > +   .get_selection  = ov772x_get_selection,
> > +   .get_fmt= ov772x_get_fmt,
> > +   .set_fmt= ov772x_set_fmt,
>
> Shouldn't these last four ops be added in the previous patch?
> They don't have anything to do with the frame interval support.
>

If you look closely you'll notice I have just re-aligned them, since I
was at there to add enum_frame_interval operation

> Anyway, after taking care of the memsets and these four ops you can add
> my:
>
> Acked-by: Hans Verkuil 

Thanks
   j

Re: [PATCH v9 03/11] media: platform: Add Renesas CEU driver

[Hans Verkuil]

On 02/21/18 13:29, Laurent Pinchart wrote:
> Hi Hans,
> 
> On Wednesday, 21 February 2018 14:03:24 EET Hans Verkuil wrote:
>> On 02/19/18 17:59, 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 | 1661 +
>>>  3 files changed, 1671 insertions(+)
>>>  create mode 100644 drivers/media/platform/renesas-ceu.c
>>
>> 
>>
>>> +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;
>>> +
>>> +   if (i >= ceudev->num_sd)
>>> +   return -EINVAL;
>>> +
>>> +   if (vb2_is_streaming(>vb2_vq))
>>> +   return -EBUSY;
>>> +
>>> +   if (i == ceudev->sd_index)
>>> +   return 0;
>>> +
>>> +   ceu_sd_old = ceudev->sd;
>>> +   ceudev->sd = >subdevs[i];
>>> +
>>> +   /* Make sure we can generate output image formats. */
>>> +   ret = ceu_init_formats(ceudev);
>>
>> Why is this done for every s_input? I would expect that this is done only
>> once for each subdev.
>>
>> I also expect to see a ceu_set_default_fmt() call here. Or that the v4l2_pix
>> is kept in ceu_subdev (i.e. per subdev) instead of a single fmt in cuedev.
>> I think I prefer that over configuring a new default format every time you
>> switch inputs.
> 
> What does userspace expect today ? I don't think we document anywhere that 
> formats are stored per-input in drivers. The VIDIOC_S_INPUT documentation is 
> very vague:
> 
> "To select a video input applications store the number of the desired input 
> in 
> an integer and call the VIDIOC_S_INPUT ioctl with a pointer to this integer. 
> Side effects are possible. For example inputs may support different video 
> standards, so the driver may implicitly switch the current standard. Because 
> of these possible side effects applications must select an input before 
> querying or negotiating any other parameters."
> 
> I understand that as meaning "anything can happen when you switch inputs, so 
> be prepared to reconfigure everything explicitly without assuming any 
> particular parameter to have a sane value".
> 
>> This code will work for two subdevs with exactly the same
>> formats/properties. But switching between e.g. a sensor and a video
>> receiver will leave things in an inconsistent state as far as I can see.
>>
>> E.g. if input 1 is the video receiver then switching to that input and
>> running 'v4l2-ctl -V' will show the sensor format, not the video receiver
>> format.
> 
> I agree that the format should be consistent with the selected input, as 
> calling VIDIOC_S_INPUT immediately followed by a stream start sequence 
> without 
> configuring formats should work (but produce a format that is not known to 
> userspace). My question is whether we should reset to a default format for 
> the 
> newly select input, or switch back to the previously set format. I'd tend to 
> go for the former to keep as little state as possible, but I'm open to 
> counter-proposals.

What to do is up to the driver. My personal preference is to make it persistent
per input, but that's just me. I won't block the other approach (resetting it
to a default). Be aware that for video receivers the format depends on the 
current
selected standard as well. I think the code does that correctly already, but it
would be good to verify if possible.

BTW, I think it is right that the spec isn't specific about what changes when
you switch inputs. It can be quite complex for drivers to handle this and it
is not unreasonable in my view for userspace to just assume it needs to 
configure
from scratch after switching inputs.

Regards,

Hans

> 
>>> +   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;
>>> +}
>>
>> The remainder of this driver looks good.
> 

Re: [PATCH v9 03/11] media: platform: Add Renesas CEU driver

[Laurent Pinchart]

Hi Hans,

On Wednesday, 21 February 2018 14:03:24 EET Hans Verkuil wrote:
> On 02/19/18 17:59, 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 | 1661 +
> >  3 files changed, 1671 insertions(+)
> >  create mode 100644 drivers/media/platform/renesas-ceu.c
> 
> 
> 
> > +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;
> > +
> > +   if (i >= ceudev->num_sd)
> > +   return -EINVAL;
> > +
> > +   if (vb2_is_streaming(>vb2_vq))
> > +   return -EBUSY;
> > +
> > +   if (i == ceudev->sd_index)
> > +   return 0;
> > +
> > +   ceu_sd_old = ceudev->sd;
> > +   ceudev->sd = >subdevs[i];
> > +
> > +   /* Make sure we can generate output image formats. */
> > +   ret = ceu_init_formats(ceudev);
> 
> Why is this done for every s_input? I would expect that this is done only
> once for each subdev.
> 
> I also expect to see a ceu_set_default_fmt() call here. Or that the v4l2_pix
> is kept in ceu_subdev (i.e. per subdev) instead of a single fmt in cuedev.
> I think I prefer that over configuring a new default format every time you
> switch inputs.

What does userspace expect today ? I don't think we document anywhere that 
formats are stored per-input in drivers. The VIDIOC_S_INPUT documentation is 
very vague:

"To select a video input applications store the number of the desired input in 
an integer and call the VIDIOC_S_INPUT ioctl with a pointer to this integer. 
Side effects are possible. For example inputs may support different video 
standards, so the driver may implicitly switch the current standard. Because 
of these possible side effects applications must select an input before 
querying or negotiating any other parameters."

I understand that as meaning "anything can happen when you switch inputs, so 
be prepared to reconfigure everything explicitly without assuming any 
particular parameter to have a sane value".

> This code will work for two subdevs with exactly the same
> formats/properties. But switching between e.g. a sensor and a video
> receiver will leave things in an inconsistent state as far as I can see.
> 
> E.g. if input 1 is the video receiver then switching to that input and
> running 'v4l2-ctl -V' will show the sensor format, not the video receiver
> format.

I agree that the format should be consistent with the selected input, as 
calling VIDIOC_S_INPUT immediately followed by a stream start sequence without 
configuring formats should work (but produce a format that is not known to 
userspace). My question is whether we should reset to a default format for the 
newly select input, or switch back to the previously set format. I'd tend to 
go for the former to keep as little state as possible, but I'm open to 
counter-proposals.

> > +   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;
> > +}
> 
> The remainder of this driver looks good.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 04/16] of: changesets: Introduce changeset helper methods

[Laurent Pinchart]

Hi Geert,

On Wednesday, 21 February 2018 12:21:50 EET Geert Uytterhoeven wrote:
> On Wed, Feb 21, 2018 at 12:10 AM, Laurent Pinchart wrote:
> > From: Pantelis Antoniou 
> > 
> > Changesets are very powerful, but the lack of a helper API
> > makes using them cumbersome. Introduce a simple copy based
> > API that makes things considerably easier.
> > 
> > To wit, adding a property using the raw API.
> > 
> > struct property *prop;
> > prop = kzalloc(sizeof(*prop)), GFP_KERNEL);
> > prop->name = kstrdup("compatible");
> > prop->value = kstrdup("foo,bar");
> > prop->length = strlen(prop->value) + 1;
> > of_changeset_add_property(ocs, np, prop);
> > 
> > while using the helper API
> > 
> > of_changeset_add_property_string(ocs, np, "compatible",
> > 
> > "foo,bar");
> > 
> > Signed-off-by: Pantelis Antoniou 
> > [Fixed memory leak in __of_changeset_add_update_property_copy()]
> > Signed-off-by: Laurent Pinchart
> > 
> 
> You missed one fix I have in my topic/overlays branch
> https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/co
> mmit/?h=topic/overlays=150f95b9dec77ce371c229f7ac4d6dd8620bef4a
> > --- a/include/linux/of.h
> > +++ b/include/linux/of.h
> > 
> > +/**
> > + * of_changeset_add_property_u32 - Create a new u32 property
> > + *
> > + * @ocs:   changeset pointer
> > + * @np:device node pointer
> > + * @name:  name of the property
> > + * @val:   value in host endian format
> > + *
> > + * Adds a u32 property to the changeset.
> > + *
> > + * Returns zero on success, a negative error value otherwise.
> > + */
> > +static inline int of_changeset_add_property_u32(struct of_changeset *ocs,
> > +   struct device_node *np, const char *name, u32 val)
> > +{
> > +   val = cpu_to_be32(val);
> 
> You must use an intermediate, to avoid complaints from sparse:
> 
> __be32 x = cpu_to_be32(val);
> 
> > +   return __of_changeset_add_update_property_copy(ocs, np, name,
> > ,
> > +   sizeof(val), false);
> 
> s/val/x/

I'll fix both in the next version, thanks.

> > +}
> > +
> > +/**
> > + * of_changeset_update_property_u32 - Update u32 property
> > + *
> > + * @ocs:   changeset pointer
> > + * @np:device node pointer
> > + * @name:  name of the property
> > + * @val:   value in host endian format
> > + *
> > + * Updates a u32 property to the changeset.
> > + *
> > + * Returns zero on success, a negative error value otherwise.
> > + */
> > +static inline int of_changeset_update_property_u32(
> > +   struct of_changeset *ocs, struct device_node *np,
> > +   const char *name, u32 val)
> > +{
> > +   val = cpu_to_be32(val);
> 
> Oh, a new one.
> 
> > +   return __of_changeset_add_update_property_copy(ocs, np, name,
> > ,
> > +   sizeof(val), true);
> > +}

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 03/16] of: dynamic: Add __of_node_dupv()

[Laurent Pinchart]

Hi Geert,

On Wednesday, 21 February 2018 12:26:45 EET Geert Uytterhoeven wrote:
> On Wed, Feb 21, 2018 at 12:10 AM, Laurent Pinchart wrote:
> > From: Pantelis Antoniou 
> > 
> > Add an __of_node_dupv() private method and make __of_node_dup() use it.
> > This is required for the subsequent changeset accessors which will
> > make use of it.
> > 
> > Signed-off-by: Pantelis Antoniou 
> > Signed-off-by: Laurent Pinchart
> > 
> > ---
> > 
> >  drivers/of/dynamic.c | 29 +++--
> >  1 file changed, 23 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
> > index 7bb33d22b4e2..4ffd04925fdf 100644
> > --- a/drivers/of/dynamic.c
> > +++ b/drivers/of/dynamic.c
> > @@ -382,8 +382,9 @@ struct property *__of_prop_dup(const struct property
> > *prop, gfp_t allocflags)> 
> >  }
> >  
> >  /**
> > 
> > - * __of_node_dup() - Duplicate or create an empty device node
> > dynamically.
> > - * @fmt: Format string (plus vargs) for new full name of the device node
> > + * __of_node_dupv() - Duplicate or create an empty device node
> > dynamically. + * @fmt: Format string for new full name of the device node
> > + * @vargs: va_list containing the arugments for the node full name
> > 
> >   *
> >   * Create an device tree node, either by duplicating an empty node or by
> >   allocating * an empty one suitable for further modification.  The node
> >   data are> 
> > @@ -391,17 +392,15 @@ struct property *__of_prop_dup(const struct property
> > *prop, gfp_t allocflags)> 
> >   * OF_DETACHED bits set. Returns the newly allocated node or NULL on out
> >   of
> >   * memory error.
> >   */
> > 
> > -struct device_node *__of_node_dup(const struct device_node *np, const
> > char *fmt, ...) +struct device_node *__of_node_dupv(const struct
> > device_node *np,
> 
> static, cfr.
> https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/co
> mmit/?h=topic/overlays=c45324e1807dd708344c9a478b777b68aca11cdf

I'll fix that in the next version.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v9 07/11] media: i2c: ov772x: Support frame interval handling

[Hans Verkuil]

On 02/19/18 17:59, Jacopo Mondi wrote:
> Add support to ov772x driver for frame intervals handling and enumeration.
> Tested with 10MHz and 24MHz input clock at VGA and QVGA resolutions for
> 10, 15 and 30 frame per second rates.
> 
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Laurent Pinchart 
> ---
>  drivers/media/i2c/ov772x.c | 212 
> +
>  1 file changed, 195 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
> index 23106d7..eba71d9 100644
> --- a/drivers/media/i2c/ov772x.c
> +++ b/drivers/media/i2c/ov772x.c
> @@ -250,6 +250,7 @@
>  #define AEC_1p2 0x10 /*  01: 1/2  window */
>  #define AEC_1p4 0x20 /*  10: 1/4  window */
>  #define AEC_2p3 0x30 /*  11: Low 2/3 window */
> +#define COM4_RESERVED   0x01 /* Reserved bit */
> 
>  /* COM5 */
>  #define AFR_ON_OFF  0x80 /* Auto frame rate control ON/OFF selection */
> @@ -267,6 +268,10 @@
>   /* AEC max step control */
>  #define AEC_NO_LIMIT0x01 /*   0 : AEC incease step has limit */
>   /*   1 : No limit to AEC increase step */
> +/* CLKRC */
> + /* Input clock divider register */
> +#define CLKRC_RESERVED  0x80 /* Reserved bit */
> +#define CLKRC_DIV(n)((n) - 1)
> 
>  /* COM7 */
>   /* SCCB Register Reset */
> @@ -373,6 +378,19 @@
>  #define VERSION(pid, ver) ((pid<<8)|(ver&0xFF))
> 
>  /*
> + * PLL multipliers
> + */
> +struct {
> + unsigned int mult;
> + u8 com4;
> +} ov772x_pll[] = {
> + { 1, PLL_BYPASS, },
> + { 4, PLL_4x, },
> + { 6, PLL_6x, },
> + { 8, PLL_8x, },
> +};
> +
> +/*
>   * struct
>   */
> 
> @@ -388,6 +406,7 @@ struct ov772x_color_format {
>  struct ov772x_win_size {
>   char *name;
>   unsigned char com7_bit;
> + unsigned int  sizeimage;
>   struct v4l2_rect  rect;
>  };
> 
> @@ -404,6 +423,7 @@ struct ov772x_priv {
>   unsigned shortflag_hflip:1;
>   /* band_filter = COM8[5] ? 256 - BDBASE : 0 */
>   unsigned shortband_filter;
> + unsigned int  fps;
>  };
> 
>  /*
> @@ -487,27 +507,35 @@ static const struct ov772x_color_format ov772x_cfmts[] 
> = {
> 
>  static const struct ov772x_win_size ov772x_win_sizes[] = {
>   {
> - .name = "VGA",
> - .com7_bit = SLCT_VGA,
> + .name   = "VGA",
> + .com7_bit   = SLCT_VGA,
> + .sizeimage  = 510 * 748,
>   .rect = {
> - .left = 140,
> - .top = 14,
> - .width = VGA_WIDTH,
> - .height = VGA_HEIGHT,
> + .left   = 140,
> + .top= 14,
> + .width  = VGA_WIDTH,
> + .height = VGA_HEIGHT,
>   },
>   }, {
> - .name = "QVGA",
> - .com7_bit = SLCT_QVGA,
> + .name   = "QVGA",
> + .com7_bit   = SLCT_QVGA,
> + .sizeimage  = 278 * 576,
>   .rect = {
> - .left = 252,
> - .top = 6,
> - .width = QVGA_WIDTH,
> - .height = QVGA_HEIGHT,
> + .left   = 252,
> + .top= 6,
> + .width  = QVGA_WIDTH,
> + .height = QVGA_HEIGHT,
>   },
>   },
>  };
> 
>  /*
> + * frame rate settings lists
> + */
> +unsigned int ov772x_frame_intervals[] = { 5, 10, 15, 20, 30, 60 };
> +#define OV772X_N_FRAME_INTERVALS ARRAY_SIZE(ov772x_frame_intervals)
> +
> +/*
>   * general function
>   */
> 
> @@ -574,6 +602,126 @@ static int ov772x_s_stream(struct v4l2_subdev *sd, int 
> enable)
>   return 0;
>  }
> 
> +static int ov772x_set_frame_rate(struct ov772x_priv *priv,
> +  struct v4l2_fract *tpf,
> +  const struct ov772x_color_format *cfmt,
> +  const struct ov772x_win_size *win)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(>subdev);
> + unsigned long fin = clk_get_rate(priv->clk);
> + unsigned int fps = tpf->numerator ?
> +tpf->denominator / tpf->numerator :
> +tpf->denominator;
> + unsigned int best_diff;
> + unsigned int fsize;
> + unsigned int pclk;
> + unsigned int diff;
> + unsigned int idx;
> + unsigned int i;
> + u8 clkrc = 0;
> + u8 com4 = 0;
> + int ret;
> +
> + /* Approximate to the closest supported frame interval. */
> + best_diff = ~0L;
> + for (i = 0, idx = 0; i < OV772X_N_FRAME_INTERVALS; i++) {
> + diff = 

Re: [PATCH v9 06/11] media: i2c: ov772x: Remove soc_camera dependencies

[Hans Verkuil]

On 02/19/18 17:59, 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 | 172 
> ++---
>  include/media/i2c/ov772x.h |   6 +-
>  4 files changed, 133 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 9f18cd2..e71e968 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 c0f94cd..b0489a1 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -68,6 +68,7 @@ obj-$(CONFIG_VIDEO_OV5670) += ov5670.o
>  obj-$(CONFIG_VIDEO_OV6650) += ov6650.o
>  obj-$(CONFIG_VIDEO_OV7640) += ov7640.o
>  obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
> +obj-$(CONFIG_VIDEO_OV772X) += ov772x.o
>  obj-$(CONFIG_VIDEO_OV7740) += ov7740.o
>  obj-$(CONFIG_VIDEO_OV9650) += ov9650.o
>  obj-$(CONFIG_VIDEO_OV13858) += ov13858.o
> diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
> index 8063835..23106d7 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 

Re: [PATCH v9 03/11] media: platform: Add Renesas CEU driver

[Hans Verkuil]

On 02/19/18 17:59, 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 | 1661 
> ++
>  3 files changed, 1671 insertions(+)
>  create mode 100644 drivers/media/platform/renesas-ceu.c
> 



> +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;
> +
> + if (i >= ceudev->num_sd)
> + return -EINVAL;
> +
> + if (vb2_is_streaming(>vb2_vq))
> + return -EBUSY;
> +
> + if (i == ceudev->sd_index)
> + return 0;
> +
> + ceu_sd_old = ceudev->sd;
> + ceudev->sd = >subdevs[i];
> +
> + /* Make sure we can generate output image formats. */
> + ret = ceu_init_formats(ceudev);

Why is this done for every s_input? I would expect that this is done only once
for each subdev.

I also expect to see a ceu_set_default_fmt() call here. Or that the v4l2_pix is 
kept
in ceu_subdev (i.e. per subdev) instead of a single fmt in cuedev. I think I 
prefer
that over configuring a new default format every time you switch inputs.

This code will work for two subdevs with exactly the same formats/properties. 
But
switching between e.g. a sensor and a video receiver will leave things in an
inconsistent state as far as I can see.

E.g. if input 1 is the video receiver then switching to that input and running
'v4l2-ctl -V' will show the sensor format, not the video receiver format.

> + 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;
> +}

The remainder of this driver looks good.

Regards,

Hans

Re: [PATCH v4 03/16] of: dynamic: Add __of_node_dupv()

[Geert Uytterhoeven]

Hi Laurent,

On Wed, Feb 21, 2018 at 12:10 AM, Laurent Pinchart
 wrote:
> From: Pantelis Antoniou 
>
> Add an __of_node_dupv() private method and make __of_node_dup() use it.
> This is required for the subsequent changeset accessors which will
> make use of it.
>
> Signed-off-by: Pantelis Antoniou 
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/of/dynamic.c | 29 +++--
>  1 file changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
> index 7bb33d22b4e2..4ffd04925fdf 100644
> --- a/drivers/of/dynamic.c
> +++ b/drivers/of/dynamic.c
> @@ -382,8 +382,9 @@ struct property *__of_prop_dup(const struct property 
> *prop, gfp_t allocflags)
>  }
>
>  /**
> - * __of_node_dup() - Duplicate or create an empty device node dynamically.
> - * @fmt: Format string (plus vargs) for new full name of the device node
> + * __of_node_dupv() - Duplicate or create an empty device node dynamically.
> + * @fmt: Format string for new full name of the device node
> + * @vargs: va_list containing the arugments for the node full name
>   *
>   * Create an device tree node, either by duplicating an empty node or by 
> allocating
>   * an empty one suitable for further modification.  The node data are
> @@ -391,17 +392,15 @@ struct property *__of_prop_dup(const struct property 
> *prop, gfp_t allocflags)
>   * OF_DETACHED bits set. Returns the newly allocated node or NULL on out of
>   * memory error.
>   */
> -struct device_node *__of_node_dup(const struct device_node *np, const char 
> *fmt, ...)
> +struct device_node *__of_node_dupv(const struct device_node *np,

static, cfr.
https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/commit/?h=topic/overlays=c45324e1807dd708344c9a478b777b68aca11cdf

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 v4 04/16] of: changesets: Introduce changeset helper methods

[Geert Uytterhoeven]

Hi Laurent,

On Wed, Feb 21, 2018 at 12:10 AM, Laurent Pinchart
 wrote:
> From: Pantelis Antoniou 
>
> Changesets are very powerful, but the lack of a helper API
> makes using them cumbersome. Introduce a simple copy based
> API that makes things considerably easier.
>
> To wit, adding a property using the raw API.
>
> struct property *prop;
> prop = kzalloc(sizeof(*prop)), GFP_KERNEL);
> prop->name = kstrdup("compatible");
> prop->value = kstrdup("foo,bar");
> prop->length = strlen(prop->value) + 1;
> of_changeset_add_property(ocs, np, prop);
>
> while using the helper API
>
> of_changeset_add_property_string(ocs, np, "compatible",
> "foo,bar");
>
> Signed-off-by: Pantelis Antoniou 
> [Fixed memory leak in __of_changeset_add_update_property_copy()]
> Signed-off-by: Laurent Pinchart 

You missed one fix I have in my topic/overlays branch
https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/commit/?h=topic/overlays=150f95b9dec77ce371c229f7ac4d6dd8620bef4a

> --- a/include/linux/of.h
> +++ b/include/linux/of.h

> +/**
> + * of_changeset_add_property_u32 - Create a new u32 property
> + *
> + * @ocs:   changeset pointer
> + * @np:device node pointer
> + * @name:  name of the property
> + * @val:   value in host endian format
> + *
> + * Adds a u32 property to the changeset.
> + *
> + * Returns zero on success, a negative error value otherwise.
> + */
> +static inline int of_changeset_add_property_u32(struct of_changeset *ocs,
> +   struct device_node *np, const char *name, u32 val)
> +{
> +   val = cpu_to_be32(val);

You must use an intermediate, to avoid complaints from sparse:

__be32 x = cpu_to_be32(val);

> +   return __of_changeset_add_update_property_copy(ocs, np, name, ,
> +   sizeof(val), false);

s/val/x/

> +}
> +
> +/**
> + * of_changeset_update_property_u32 - Update u32 property
> + *
> + * @ocs:   changeset pointer
> + * @np:device node pointer
> + * @name:  name of the property
> + * @val:   value in host endian format
> + *
> + * Updates a u32 property to the changeset.
> + *
> + * Returns zero on success, a negative error value otherwise.
> + */
> +static inline int of_changeset_update_property_u32(
> +   struct of_changeset *ocs, struct device_node *np,
> +   const char *name, u32 val)
> +{
> +   val = cpu_to_be32(val);

Oh, a new one.

> +   return __of_changeset_add_update_property_copy(ocs, np, name, ,
> +   sizeof(val), true);
> +}

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 19/19] ARM64: dts: r8a77965: Add EtherAVB device node

[jacopo mondi]

Hi Sergei,

On Tue, Feb 20, 2018 at 06:30:56PM +0300, Sergei Shtylyov wrote:
> On 02/20/2018 06:12 PM, Jacopo Mondi wrote:
>
> > Populate the ethernet@e680 device node to enable Ethernet interface
> > for R-Car M3-N (r8a77965) SoC.
> >
> > Signed-off-by: Jacopo Mondi 
> > Reviewed-by: Geert Uytterhoeven 
> >
> > ---
> > v1 -> v2:
> > - Replace ALWAYS_ON power area identifier with numeric constant
> > ---
> >  arch/arm64/boot/dts/renesas/r8a77965.dtsi | 43 
> > ++-
> >  1 file changed, 42 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi 
> > b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> > index 55f05f7..c249895 100644
> > --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> > @@ -520,7 +520,48 @@
> > };
> >
> > avb: ethernet@e680 {
> > -   /* placeholder */
> > +   compatible = "renesas,etheravb-r8a77965",
> > +"renesas,etheravb-rcar-gen3";
> > +   reg = <0 0xe680 0 0x800>, <0 0xe6a0 0 0x1>;
> > +   interrupts = ,
> > +,
> > +,
> > +,
> > +,
> > +,
> > +,
> > +,
> > +,
> > +,
> > +,
> > +,
> > +,
> > +,
> > +,
> > +,
> > +,
> > +,
> > +,
> > +,
> > +,
> > +,
> > +,
> > +,
> > +;
> > +   interrupt-names = "ch0", "ch1", "ch2", "ch3",
> > + "ch4", "ch5", "ch6", "ch7",
> > + "ch8", "ch9", "ch10", "ch11",
> > + "ch12", "ch13", "ch14", "ch15",
> > + "ch16", "ch17", "ch18", "ch19",
> > + "ch20", "ch21", "ch22", "ch23",
> > + "ch24";
> > +   clocks = < CPG_MOD 812>;
> > +   power-domains = < 32>;
> > +   resets = < 812>;
> > +   phy-mode = "rgmii-txid";
>
>Why not just "rgmii"? TX delay is a board specific detail, no?
>

I admit I took this one straight from r8a7796 dtsi.
Would you like to me resend and change this?

Thanks
   j

> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +   status = "disabled";
> > };
> >
> > csi20: csi2@fea8 {
> >
>
> MBR, Sergei

Re: [PATCH v4 01/16] dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings

[Sergei Shtylyov]

On 2/21/2018 2:10 AM, Laurent Pinchart wrote:


The Renesas R-Car Gen2 and Gen3 SoCs have internal LVDS encoders. Add
corresponding device tree bindings.

Signed-off-by: Laurent Pinchart 
Reviewed-by: Rob Herring 
---
Changes since v1:

- Move the SoC name before the IP name in compatible strings
- Rename parallel input to parallel RGB input
- Fixed "renesas,r8a7743-lvds" description
- Document the resets property
- Fixed typo
---
  .../bindings/display/bridge/renesas,lvds.txt   | 56 ++
  MAINTAINERS|  1 +
  2 files changed, 57 insertions(+)
  create mode 100644 
Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt

diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt 
b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
new file mode 100644
index ..2b19ce51ec07
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
@@ -0,0 +1,56 @@
+Renesas R-Car LVDS Encoder
+==
+
+These DT bindings describe the LVDS encoder embedded in the Renesas R-Car
+Gen2, R-Car Gen3 and RZ/G SoCs.
+
+Required properties:
+
+- compatible : Shall contain one of
+  - "renesas,r8a7743-lvds" for R8A7743 (RZ/G1M) compatible LVDS encoders
+  - "renesas,r8a7790-lvds" for R8A7790 (R-Car H2) compatible LVDS encoders
+  - "renesas,r8a7791-lvds" for R8A7791 (R-Car M2-W) compatible LVDS encoders
+  - "renesas,r8a7793-lvds" for R8A7791 (R-Car M2-N) compatible LVDS encoders
+  - "renesas,r8a7795-lvds" for R8A7795 (R-Car H3) compatible LVDS encoders
+  - "renesas,r8a7796-lvds" for R8A7796 (R-Car M3-W) compatible LVDS encoders
+
+- reg: Base address and length for the memory-mapped registers
+- clocks: A phandle + clock-specifier pair for the functional clock
+- resets: A phandle + reset specifier for the module reset
+
+Required nodes:
+
+The LVDS encoder has two video ports. Their connections are modelled using the
+OF graph bindings specified in Documentation/devicetree/bindings/graph.txt.
+
+- Video port 0 corresponds to the parallel RGB input
+- Video port 1 corresponds to the LVDS output
+
+Each port shall have a single endpoint.
+
+
+Example:
+
+   lvds0: lvds@feb9 {


   "lvds-encoder@feb9" maybe? And do we need a # in the label if the 
encoder is alone in DT anyway?


[...]

MBR, Sergei

Re: [PATCH v4 14/16] ARM: dts: r8a7794: Convert to new DU DT bindings

[Sergei Shtylyov]

Hello!

On 2/21/2018 2:10 AM, Laurent Pinchart wrote:


The DU DT bindings have been updated to drop the reg-names property.
Update the r8a7792 device tree accordingly.


   Apparently r8a7794.


Signed-off-by: Laurent Pinchart 

[...]

MBR, Sergei

Re: [PATCH v4 07/16] i2c: demux: Use changeset helpers for clarity

[Wolfram Sang]

On Wed, Feb 21, 2018 at 01:10:37AM +0200, Laurent Pinchart wrote:
> From: Pantelis Antoniou 
> 
> The changeset helpers are easier to use, use them instead of
> using the static property.
> 
> Signed-off-by: Pantelis Antoniou 
> Acked-by: Wolfram Sang 

My ack still holds. Good luck pushing this series!



signature.asc
Description: PGP signature