devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 00/16] R-Car DU: Convert LVDS code to bridge driver
@ 2018-02-20 23:10 Laurent Pinchart
  2018-02-20 23:10 ` [PATCH v4 01/16] dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings Laurent Pinchart
                   ` (9 more replies)
  0 siblings, 10 replies; 33+ messages in thread
From: Laurent Pinchart @ 2018-02-20 23:10 UTC (permalink / raw)
  To: dri-devel
  Cc: Marek Vasut, Wolfram Sang, Koen Kooi, Pantelis Antoniou,
	linux-kernel, linux-renesas-soc, devicetree, linux-i2c,
	Matt Porter, Frank Rowand, Guenter Roeck

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

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 +-
 arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts     |   3 +-
 arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts |   3 +-
 .../arm64/boot/dts/renesas/r8a7795-salvator-xs.dts |   3 +-
 arch/arm64/boot/dts/renesas/r8a7795.dtsi           |  36 +-
 arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts     |   3 +-
 arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts |   3 +-
 arch/arm64/boot/dts/renesas/r8a7796.dtsi           |  36 +-
 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    |  81 ++++
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts    |  55 +++
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts    |  55 +++
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts    |  55 +++
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts    |  55 +++
 drivers/gpu/drm/rcar-du/rcar_lvds.c                | 524 +++++++++++++++++++++
 drivers/i2c/muxes/i2c-demux-pinctrl.c              |  12 +-
 drivers/of/dynamic.c                               | 317 ++++++++++++-
 drivers/of/unittest.c                              |  54 +++
 include/linux/of.h                                 | 337 +++++++++++++
 43 files changed, 2199 insertions(+), 710 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
 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_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
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_lvds.c

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v4 01/16] dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings
  2018-02-20 23:10 [PATCH v4 00/16] R-Car DU: Convert LVDS code to bridge driver Laurent Pinchart
@ 2018-02-20 23:10 ` Laurent Pinchart
  2018-02-21  8:35   ` Sergei Shtylyov
  2018-02-20 23:10 ` [PATCH v4 02/16] dt-bindings: display: renesas: Deprecate LVDS support in the DU bindings Laurent Pinchart
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 33+ messages in thread
From: Laurent Pinchart @ 2018-02-20 23:10 UTC (permalink / raw)
  To: dri-devel; +Cc: linux-renesas-soc, devicetree

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

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
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 000000000000..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@feb90000 {
+		compatible = "renesas,r8a7790-lvds";
+		reg = <0 0xfeb90000 0 0x1c>;
+		clocks = <&cpg CPG_MOD 726>;
+		resets = <&cpg 726>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+				lvds0_in: endpoint {
+					remote-endpoint = <&du_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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 02/16] dt-bindings: display: renesas: Deprecate LVDS support in the DU bindings
  2018-02-20 23:10 [PATCH v4 00/16] R-Car DU: Convert LVDS code to bridge driver Laurent Pinchart
  2018-02-20 23:10 ` [PATCH v4 01/16] dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings Laurent Pinchart
@ 2018-02-20 23:10 ` Laurent Pinchart
  2018-02-20 23:10 ` [PATCH v4 03/16] of: dynamic: Add __of_node_dupv() Laurent Pinchart
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 33+ messages in thread
From: Laurent Pinchart @ 2018-02-20 23:10 UTC (permalink / raw)
  To: dri-devel; +Cc: linux-renesas-soc, devicetree

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

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
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@feb00000 {
 		compatible = "renesas,du-r8a7795";
-		reg = <0 0xfeb00000 0 0x80000>,
-		      <0 0xfeb90000 0 0x14>;
-		reg-names = "du", "lvds.0";
+		reg = <0 0xfeb00000 0 0x80000>;
 		interrupts = <GIC_SPI 256 IRQ_TYPE_LEVEL_HIGH>,
 			     <GIC_SPI 268 IRQ_TYPE_LEVEL_HIGH>,
 			     <GIC_SPI 269 IRQ_TYPE_LEVEL_HIGH>,
@@ -79,9 +71,8 @@ Example: R8A7795 (R-Car H3) ES2.0 DU
 		clocks = <&cpg CPG_MOD 724>,
 			 <&cpg CPG_MOD 723>,
 			 <&cpg CPG_MOD 722>,
-			 <&cpg CPG_MOD 721>,
-			 <&cpg CPG_MOD 727>;
-		clock-names = "du.0", "du.1", "du.2", "du.3", "lvds.0";
+			 <&cpg CPG_MOD 721>;
+		clock-names = "du.0", "du.1", "du.2", "du.3";
 		vsps = <&vspd0 0>, <&vspd1 0>, <&vspd2 0>, <&vspd0 1>;
 
 		ports {
-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 03/16] of: dynamic: Add __of_node_dupv()
  2018-02-20 23:10 [PATCH v4 00/16] R-Car DU: Convert LVDS code to bridge driver Laurent Pinchart
  2018-02-20 23:10 ` [PATCH v4 01/16] dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings Laurent Pinchart
  2018-02-20 23:10 ` [PATCH v4 02/16] dt-bindings: display: renesas: Deprecate LVDS support in the DU bindings Laurent Pinchart
@ 2018-02-20 23:10 ` Laurent Pinchart
  2018-02-21 10:26   ` Geert Uytterhoeven
  2018-02-21 23:16   ` Rob Herring
  2018-02-20 23:10 ` [PATCH v4 04/16] of: changesets: Introduce changeset helper methods Laurent Pinchart
                   ` (6 subsequent siblings)
  9 siblings, 2 replies; 33+ messages in thread
From: Laurent Pinchart @ 2018-02-20 23:10 UTC (permalink / raw)
  To: dri-devel
  Cc: Marek Vasut, Wolfram Sang, Koen Kooi, Pantelis Antoniou,
	linux-kernel, linux-renesas-soc, devicetree, linux-i2c,
	Matt Porter, Frank Rowand, Guenter Roeck

From: Pantelis Antoniou <pantelis.antoniou@konsulko.com>

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 <pantelis.antoniou@konsulko.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 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,
+		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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 04/16] of: changesets: Introduce changeset helper methods
  2018-02-20 23:10 [PATCH v4 00/16] R-Car DU: Convert LVDS code to bridge driver Laurent Pinchart
                   ` (2 preceding siblings ...)
  2018-02-20 23:10 ` [PATCH v4 03/16] of: dynamic: Add __of_node_dupv() Laurent Pinchart
@ 2018-02-20 23:10 ` Laurent Pinchart
  2018-02-21 10:21   ` Geert Uytterhoeven
  2018-02-21 23:14   ` Rob Herring
  2018-02-20 23:10 ` [PATCH v4 05/16] of: changeset: Add of_changeset_node_move method Laurent Pinchart
                   ` (5 subsequent siblings)
  9 siblings, 2 replies; 33+ messages in thread
From: Laurent Pinchart @ 2018-02-20 23:10 UTC (permalink / raw)
  To: dri-devel
  Cc: Marek Vasut, Wolfram Sang, Koen Kooi, Pantelis Antoniou,
	linux-kernel, linux-renesas-soc, devicetree, linux-i2c,
	Matt Porter, Frank Rowand, Guenter Roeck

From: Pantelis Antoniou <pantelis.antoniou@konsulko.com>

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 <pantelis.antoniou@konsulko.com>
[Fixed memory leak in __of_changeset_add_update_property_copy()]
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/of/dynamic.c | 222 ++++++++++++++++++++++++++++++++++
 include/linux/of.h   | 328 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 550 insertions(+)

diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 4ffd04925fdf..85e722ed8631 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);
+	kfree(prop);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(__of_changeset_add_update_property_copy);
+
+/**
+ * of_changeset_add_property_stringf - Create a new formatted string property
+ *
+ * @ocs:	changeset pointer
+ * @np:		device node pointer
+ * @name:	name of the property
+ * @fmt:	format of string property
+ * ...		arguments of the format string
+ *
+ * Adds a string property to the changeset by making copies of the name
+ * and the formatted value.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+__printf(4, 5) int of_changeset_add_property_stringf(
+		struct of_changeset *ocs, struct device_node *np,
+		const char *name, const char *fmt, ...)
+{
+	va_list vargs;
+	int ret;
+
+	va_start(vargs, fmt);
+	ret = __of_changeset_add_update_property_stringv(ocs, np, name, fmt,
+			vargs, false);
+	va_end(vargs);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(of_changeset_add_property_stringf);
+
+/**
+ * of_changeset_update_property_stringf - Update formatted string property
+ *
+ * @ocs:	changeset pointer
+ * @np:		device node pointer
+ * @name:	name of the property
+ * @fmt:	format of string property
+ * ...		arguments of the format string
+ *
+ * Updates a string property to the changeset by making copies of the name
+ * and the formatted value.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+int of_changeset_update_property_stringf(
+	struct of_changeset *ocs, struct device_node *np,
+	const char *name, const char *fmt, ...)
+{
+	va_list vargs;
+	int ret;
+
+	va_start(vargs, fmt);
+	ret = __of_changeset_add_update_property_stringv(ocs, np, name, fmt,
+			vargs, true);
+	va_end(vargs);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(of_changeset_update_property_stringf);
+
+/**
+ * __of_changeset_add_update_property_string_list - Create/update a string
+ *                                                  list property
+ *
+ * @ocs:	changeset pointer
+ * @np:		device node pointer
+ * @name:	name of the property
+ * @strs:	pointer to the string list
+ * @count:	string count
+ * @update:	True on update operation
+ *
+ * Adds a string list property to the changeset.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+int __of_changeset_add_update_property_string_list(
+		struct of_changeset *ocs, struct device_node *np,
+		const char *name, const char **strs, int count, bool update)
+{
+	int total = 0, i, ret;
+	char *value, *s;
+
+	for (i = 0; i < count; i++) {
+		/* check if  it's NULL */
+		if (!strs[i])
+			return -EINVAL;
+		total += strlen(strs[i]) + 1;
+	}
+
+	value = kmalloc(total, GFP_KERNEL);
+	if (!value)
+		return -ENOMEM;
+
+	for (i = 0, s = value; i < count; i++) {
+		/* no need to check for NULL, check above */
+		strcpy(s, strs[i]);
+		s += strlen(strs[i]) + 1;
+	}
+
+	ret = __of_changeset_add_update_property_copy(ocs, np, name, value,
+			total, update);
+
+	kfree(value);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(__of_changeset_add_update_property_string_list);
diff --git a/include/linux/of.h b/include/linux/of.h
index da1ee95241c1..7aef555f9bc2 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -1266,6 +1266,8 @@ enum of_reconfig_change {
 };
 
 #ifdef CONFIG_OF_DYNAMIC
+#include <linux/slab.h>
+
 extern int of_reconfig_notifier_register(struct notifier_block *);
 extern int of_reconfig_notifier_unregister(struct notifier_block *);
 extern int of_reconfig_notify(unsigned long, struct of_reconfig_data *rd);
@@ -1309,6 +1311,23 @@ static inline int of_changeset_update_property(struct of_changeset *ocs,
 {
 	return of_changeset_action(ocs, OF_RECONFIG_UPDATE_PROPERTY, np, prop);
 }
+
+struct device_node *of_changeset_create_device_nodev(
+	struct of_changeset *ocs, struct device_node *parent,
+	const char *fmt, va_list vargs);
+
+__printf(3, 4) struct device_node *
+of_changeset_create_device_node(struct of_changeset *ocs,
+	struct device_node *parent, const char *fmt, ...);
+
+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);
+
+int __of_changeset_add_update_property_string_list(
+		struct of_changeset *ocs, struct device_node *np,
+		const char *name, const char **strs, int count, bool update);
+
 #else /* CONFIG_OF_DYNAMIC */
 static inline int of_reconfig_notifier_register(struct notifier_block *nb)
 {
@@ -1328,8 +1347,317 @@ static inline int of_reconfig_get_state_change(unsigned long action,
 {
 	return -EINVAL;
 }
+
+static inline struct device_node *of_changeset_create_device_nodev(
+	struct of_changeset *ocs, struct device_node *parent,
+	const char *fmt, va_list vargs)
+{
+	return ERR_PTR(-EINVAL);
+}
+
+static inline __printf(3, 4) struct device_node *
+of_changeset_create_device_node(struct of_changeset *ocs,
+	struct device_node *parent, const char *fmt, ...)
+{
+	return ERR_PTR(-EINVAL);
+}
+
+static inline 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)
+{
+	return -EINVAL;
+}
+
+static inline __printf(4, 5) int of_changeset_add_property_stringf(
+		struct of_changeset *ocs, struct device_node *np,
+		const char *name, const char *fmt, ...)
+{
+	return -EINVAL;
+}
+
+static inline int of_changeset_update_property_stringf(
+	struct of_changeset *ocs, struct device_node *np,
+	const char *name, const char *fmt, ...)
+{
+	return -EINVAL;
+}
+
+static inline int __of_changeset_add_update_property_string_list(
+		struct of_changeset *ocs, struct device_node *np,
+		const char *name, const char **strs, int count, bool update)
+{
+	return -EINVAL;
+}
+
 #endif /* CONFIG_OF_DYNAMIC */
 
+/**
+ * of_changeset_add_property_copy - Create 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
+ *
+ * Adds a property to the changeset by making copies of the name & value
+ * entries.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+static inline int of_changeset_add_property_copy(struct of_changeset *ocs,
+	struct device_node *np, const char *name,
+	const void *value, int length)
+{
+	return __of_changeset_add_update_property_copy(ocs, np, name, value,
+			length, false);
+}
+
+/**
+ * of_changeset_update_property_copy - Update a 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 a property to the changeset by making copies of the name & value
+ * entries.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+static inline int of_changeset_update_property_copy(struct of_changeset *ocs,
+	struct device_node *np, const char *name,
+	const void *value, int length)
+{
+	return __of_changeset_add_update_property_copy(ocs, np, name, value,
+			length, true);
+}
+
+/**
+ * __of_changeset_add_update_property_string - Create/update a string property
+ *
+ * @ocs:	changeset pointer
+ * @np:		device node pointer
+ * @name:	name of the property
+ * @str:	string property value
+ * @update:	True on update operation
+ *
+ * Adds/updates a string property to the changeset by making copies of the name
+ * and the given value. The @update parameter controls whether an add or
+ * update takes place.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+static inline int __of_changeset_add_update_property_string(
+	struct of_changeset *ocs, struct device_node *np, const char *name,
+	const char *str, bool update)
+{
+	return __of_changeset_add_update_property_copy(ocs, np, name, str,
+			strlen(str) + 1, update);
+}
+
+/**
+ * __of_changeset_add_update_property_stringv - Create/update a formatted
+ *						string property
+ *
+ * @ocs:	changeset pointer
+ * @np:		device node pointer
+ * @name:	name of the property
+ * @fmt:	format of string property
+ * @vargs:	arguments of the format string
+ * @update:	True on update operation
+ *
+ * Adds/updates a string property to the changeset by making copies of the name
+ * and the formatted value. The @update parameter controls whether an add or
+ * update takes place.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+static inline int __of_changeset_add_update_property_stringv(
+	struct of_changeset *ocs, struct device_node *np, const char *name,
+	const char *fmt, va_list vargs, bool update)
+{
+	char *str;
+	int ret;
+
+	str = kvasprintf(GFP_KERNEL, fmt, vargs);
+	if (!str)
+		return -ENOMEM;
+	ret = __of_changeset_add_update_property_string(ocs, np, name, str,
+			update);
+	kfree(str);
+
+	return ret;
+}
+
+/**
+ * of_changeset_add_property_string_list - Create a new string list property
+ *
+ * @ocs:	changeset pointer
+ * @np:		device node pointer
+ * @name:	name of the property
+ * @strs:	pointer to the string list
+ * @count:	string count
+ *
+ * Adds a string list property to the changeset.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+static inline int of_changeset_add_property_string_list(
+	struct of_changeset *ocs, struct device_node *np, const char *name,
+	const char **strs, int count)
+{
+	return __of_changeset_add_update_property_string_list(ocs, np, name,
+			strs, count, false);
+}
+
+/**
+ * of_changeset_update_property_string_list - Update string list property
+ *
+ * @ocs:	changeset pointer
+ * @np:		device node pointer
+ * @name:	name of the property
+ * @strs:	pointer to the string list
+ * @count:	string count
+ *
+ * Updates a string list property to the changeset.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+static inline int of_changeset_update_property_string_list(
+	struct of_changeset *ocs, struct device_node *np,
+	const char *name, const char **strs, int count)
+{
+	return __of_changeset_add_update_property_string_list(ocs, np, name,
+			strs, count, true);
+}
+
+/**
+ * of_changeset_add_property_string - Adds a string property
+ *
+ * @ocs:	changeset pointer
+ * @np:		device node pointer
+ * @name:	name of the property
+ * @str:	string property
+ *
+ * Adds a string property to the changeset by making copies of the name
+ * and the string value.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+static inline int of_changeset_add_property_string(
+	struct of_changeset *ocs, struct device_node *np,
+	const char *name, const char *str)
+{
+	return __of_changeset_add_update_property_string(ocs, np, name, str,
+			false);
+}
+
+/**
+ * of_changeset_update_property_string - Update a string property
+ *
+ * @ocs:	changeset pointer
+ * @np:		device node pointer
+ * @name:	name of the property
+ * @str:	string property
+ *
+ * Updates a string property to the changeset by making copies of the name
+ * and the string value.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+static inline int of_changeset_update_property_string(
+	struct of_changeset *ocs, struct device_node *np,
+	const char *name, const char *str)
+{
+	return __of_changeset_add_update_property_string(ocs, np, name, str,
+			true);
+}
+
+/**
+ * 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);
+	return __of_changeset_add_update_property_copy(ocs, np, name, &val,
+			sizeof(val), false);
+}
+
+/**
+ * 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);
+	return __of_changeset_add_update_property_copy(ocs, np, name, &val,
+			sizeof(val), true);
+}
+
+/**
+ * of_changeset_add_property_bool - Create a new u32 property
+ *
+ * @ocs:	changeset pointer
+ * @np:		device node pointer
+ * @name:	name of the property
+ *
+ * Adds a bool property to the changeset. Note that there is
+ * no option to set the value to false, since the property
+ * existing sets it to true.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+static inline int of_changeset_add_property_bool(
+	struct of_changeset *ocs, struct device_node *np, const char *name)
+{
+	return __of_changeset_add_update_property_copy(ocs, np, name, "", 0,
+			false);
+}
+
+/**
+ * of_changeset_update_property_bool - Update a bool property
+ *
+ * @ocs:	changeset pointer
+ * @np:		device node pointer
+ * @name:	name of the property
+ *
+ * Updates a property to the changeset. Note that there is
+ * no option to set the value to false, since the property
+ * existing sets it to true.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+static inline int of_changeset_update_property_bool(struct of_changeset *ocs,
+		struct device_node *np, const char *name)
+{
+	return __of_changeset_add_update_property_copy(ocs, np, name, "", 0,
+			true);
+}
+
 /**
  * of_device_is_system_power_controller - Tells if system-power-controller is found for device_node
  * @np: Pointer to the given device_node
-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 05/16] of: changeset: Add of_changeset_node_move method
  2018-02-20 23:10 [PATCH v4 00/16] R-Car DU: Convert LVDS code to bridge driver Laurent Pinchart
                   ` (3 preceding siblings ...)
  2018-02-20 23:10 ` [PATCH v4 04/16] of: changesets: Introduce changeset helper methods Laurent Pinchart
@ 2018-02-20 23:10 ` Laurent Pinchart
  2018-02-21 23:20   ` Rob Herring
  2018-02-20 23:10 ` [PATCH v4 06/16] of: unittest: changeset helpers Laurent Pinchart
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 33+ messages in thread
From: Laurent Pinchart @ 2018-02-20 23:10 UTC (permalink / raw)
  To: dri-devel
  Cc: Marek Vasut, Wolfram Sang, Koen Kooi, Pantelis Antoniou,
	linux-kernel, linux-renesas-soc, devicetree, linux-i2c,
	Matt Porter, Frank Rowand, Guenter Roeck

From: Pantelis Antoniou <pantelis.antoniou@konsulko.com>

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.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/of/dynamic.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of.h   |  9 +++++++
 2 files changed, 75 insertions(+)

diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 85e722ed8631..27d9057ef360 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -1132,3 +1132,69 @@ int __of_changeset_add_update_property_string_list(
 	return ret;
 }
 EXPORT_SYMBOL_GPL(__of_changeset_add_update_property_string_list);
+
+static struct device_node *
+__of_changeset_node_move_one(struct of_changeset *ocs,
+		struct device_node *np, struct device_node *new_parent)
+{
+	struct device_node *np2;
+	const char *unitname;
+	int err;
+
+	err = of_changeset_detach_node(ocs, np);
+	if (err)
+		return ERR_PTR(err);
+
+	unitname = strrchr(np->full_name, '/');
+	if (!unitname)
+		unitname = np->full_name;
+
+	np2 = __of_node_dup(np, "%s/%s",
+			new_parent->full_name, unitname);
+	if (!np2)
+		return ERR_PTR(-ENOMEM);
+	np2->parent = new_parent;
+
+	err = of_changeset_attach_node(ocs, np2);
+	if (err)
+		return ERR_PTR(err);
+
+	return np2;
+}
+
+/**
+ * of_changeset_node_move_to - Moves a subtree to a new place in
+ *                             the tree
+ *
+ * @ocs:	changeset pointer
+ * @np:		device node pointer to be moved
+ * @to:		device node of the new parent
+ *
+ * Moves a subtree to a new place in the tree.
+ * Note that a move is a safe operation because the phandles
+ * remain valid.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+int of_changeset_node_move(struct of_changeset *ocs,
+		struct device_node *np, struct device_node *new_parent)
+{
+	struct device_node *npc, *nppc;
+
+	/* move the root first */
+	nppc = __of_changeset_node_move_one(ocs, np, new_parent);
+	if (IS_ERR(nppc))
+		return PTR_ERR(nppc);
+
+	/* move the subtrees next */
+	for_each_child_of_node(np, npc) {
+		nppc = __of_changeset_node_move_one(ocs, npc, nppc);
+		if (IS_ERR(nppc)) {
+			of_node_put(npc);
+			return PTR_ERR(nppc);
+		}
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_changeset_node_move);
diff --git a/include/linux/of.h b/include/linux/of.h
index 7aef555f9bc2..76197bc75346 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -1328,6 +1328,9 @@ int __of_changeset_add_update_property_string_list(
 		struct of_changeset *ocs, struct device_node *np,
 		const char *name, const char **strs, int count, bool update);
 
+int of_changeset_node_move(struct of_changeset *ocs,
+	struct device_node *np, struct device_node *new_parent);
+
 #else /* CONFIG_OF_DYNAMIC */
 static inline int of_reconfig_notifier_register(struct notifier_block *nb)
 {
@@ -1390,6 +1393,12 @@ static inline int __of_changeset_add_update_property_string_list(
 	return -EINVAL;
 }
 
+static inline int of_changeset_node_move(struct of_changeset *ocs,
+		struct device_node *np, struct device_node *new_parent)
+{
+	return -EINVAL;
+}
+
 #endif /* CONFIG_OF_DYNAMIC */
 
 /**
-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 06/16] of: unittest: changeset helpers
  2018-02-20 23:10 [PATCH v4 00/16] R-Car DU: Convert LVDS code to bridge driver Laurent Pinchart
                   ` (4 preceding siblings ...)
  2018-02-20 23:10 ` [PATCH v4 05/16] of: changeset: Add of_changeset_node_move method Laurent Pinchart
@ 2018-02-20 23:10 ` Laurent Pinchart
  2018-02-21 23:10   ` Rob Herring
  2018-02-20 23:10 ` [PATCH v4 07/16] i2c: demux: Use changeset helpers for clarity Laurent Pinchart
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 33+ messages in thread
From: Laurent Pinchart @ 2018-02-20 23:10 UTC (permalink / raw)
  To: dri-devel
  Cc: Marek Vasut, Wolfram Sang, Koen Kooi, Pantelis Antoniou,
	linux-kernel, linux-renesas-soc, devicetree, linux-i2c,
	Matt Porter, Frank Rowand, Guenter Roeck

From: Pantelis Antoniou <pantelis.antoniou@konsulko.com>

Add a unitest specific for the new changeset helpers.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 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(&chgset);
+
+	parent = of_find_node_by_path("/testcase-data/changeset");
+
+	unittest(parent, "testcase setup failure\n");
+	n1 = of_changeset_create_device_node(&chgset,
+			parent, "/testcase-data/changeset/n1");
+	unittest(n1, "testcase setup failure\n");
+	n2 = of_changeset_create_device_node(&chgset,
+			parent, "/testcase-data/changeset/n2");
+	unittest(n2, "testcase setup failure\n");
+	n21 = of_changeset_create_device_node(&chgset, n2, "%s/%s",
+			"/testcase-data/changeset/n2", "n21");
+	unittest(n21, "testcase setup failure\n");
+
+	unittest(!of_changeset_add_property_string(&chgset, parent,
+				"prop-add", "foo"), "fail add prop\n");
+
+	unittest(!of_changeset_attach_node(&chgset, n1), "fail n1 attach\n");
+	unittest(!of_changeset_attach_node(&chgset, n2), "fail n2 attach\n");
+	unittest(!of_changeset_attach_node(&chgset, n21), "fail n21 attach\n");
+
+	unittest(!of_changeset_apply(&chgset), "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(&chgset), "revert failed\n");
+
+	of_changeset_destroy(&chgset);
+
+	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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 07/16] i2c: demux: Use changeset helpers for clarity
  2018-02-20 23:10 [PATCH v4 00/16] R-Car DU: Convert LVDS code to bridge driver Laurent Pinchart
                   ` (5 preceding siblings ...)
  2018-02-20 23:10 ` [PATCH v4 06/16] of: unittest: changeset helpers Laurent Pinchart
@ 2018-02-20 23:10 ` Laurent Pinchart
  2018-02-21  8:06   ` Wolfram Sang
  2018-02-20 23:10 ` [PATCH v4 08/16] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes Laurent Pinchart
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 33+ messages in thread
From: Laurent Pinchart @ 2018-02-20 23:10 UTC (permalink / raw)
  To: dri-devel
  Cc: Marek Vasut, Wolfram Sang, Koen Kooi, Pantelis Antoniou,
	linux-kernel, linux-renesas-soc, devicetree, linux-i2c,
	Matt Porter, Frank Rowand, Guenter Roeck

From: Pantelis Antoniou <pantelis.antoniou@konsulko.com>

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

Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
Acked-by: Wolfram Sang <wsa@the-dreams.de>
["okay" -> "ok"]
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 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(&pdev->dev, sizeof(*priv)
 			   + num_chan * sizeof(struct i2c_demux_pinctrl_chan), GFP_KERNEL);
-
-	props = devm_kcalloc(&pdev->dev, num_chan, sizeof(*props), GFP_KERNEL);
-
-	if (!priv || !props)
+	if (!priv)
 		return -ENOMEM;
 
 	err = of_property_read_string(np, "i2c-bus-name", &priv->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(&pdev->dev, "status", GFP_KERNEL);
-		props[i].value = devm_kstrdup(&pdev->dev, "ok", GFP_KERNEL);
-		props[i].length = 3;
-
 		of_changeset_init(&priv->chan[i].chgset);
-		of_changeset_update_property(&priv->chan[i].chgset, adap_np, &props[i]);
+		of_changeset_update_property_string(&priv->chan[i].chgset,
+						    adap_np, "status", "ok");
 	}
 
 	priv->num_chan = num_chan;
-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 08/16] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes
  2018-02-20 23:10 [PATCH v4 00/16] R-Car DU: Convert LVDS code to bridge driver Laurent Pinchart
                   ` (6 preceding siblings ...)
  2018-02-20 23:10 ` [PATCH v4 07/16] i2c: demux: Use changeset helpers for clarity Laurent Pinchart
@ 2018-02-20 23:10 ` Laurent Pinchart
  2018-02-21 23:28   ` Rob Herring
  2018-02-21 16:39 ` [PATCH v4 00/16] R-Car DU: Convert LVDS code to bridge driver Simon Horman
  2018-02-22  6:07 ` Frank Rowand
  9 siblings, 1 reply; 33+ messages in thread
From: Laurent Pinchart @ 2018-02-20 23:10 UTC (permalink / raw)
  To: dri-devel; +Cc: linux-renesas-soc, Frank Rowand, devicetree

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 <laurent.pinchart+renesas@ideasonboard.com>
---
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 <soc>-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    |  81 ++++++
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts    |  55 ++++
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts    |  55 ++++
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts    |  55 ++++
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts    |  55 ++++
 9 files changed, 636 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 000000000000..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 <laurent.pinchart@ideasonboard.com>
+ *
+ * Based on work from Jyri Sarha <jsarha@ti.com>
+ * Copyright (C) 2015 Texas Instruments
+ */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_fdt.h>
+#include <linux/of_graph.h>
+#include <linux/slab.h>
+
+#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 = __dtb_rcar_du_of_##type##_##soc##_begin,	\
+		.end = __dtb_rcar_du_of_##type##_##soc##_end,		\
+	}
+
+static int __init rcar_du_of_apply_overlay(const struct rcar_du_of_overlay *dtbs,
+					   const char *compatible)
+{
+	const struct rcar_du_of_overlay *dtb = NULL;
+	struct device_node *node = NULL;
+	unsigned int i;
+	int ovcs_id;
+	void *data;
+	void *mem;
+	int ret;
+
+	for (i = 0; dtbs[i].compatible; ++i) {
+		if (!strcmp(dtbs[i].compatible, compatible)) {
+			dtb = &dtbs[i];
+			break;
+		}
+	}
+
+	if (!dtb)
+		return -ENODEV;
+
+	data = kmemdup(dtb->begin, dtb->end - dtb->begin, GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	mem = of_fdt_unflatten_tree(data, NULL, &node);
+	if (!mem) {
+		ret = -ENOMEM;
+		goto done;
+	}
+
+	ovcs_id = 0;
+	ret = of_overlay_apply(node, &ovcs_id);
+
+done:
+	of_node_put(node);
+	kfree(data);
+	kfree(mem);
+
+	return ret;
+}
+
+/* -----------------------------------------------------------------------------
+ * LVDS Overlays
+ */
+
+RCAR_DU_OF_DTB(lvds, r8a7790);
+RCAR_DU_OF_DTB(lvds, r8a7791);
+RCAR_DU_OF_DTB(lvds, r8a7793);
+RCAR_DU_OF_DTB(lvds, r8a7795);
+RCAR_DU_OF_DTB(lvds, r8a7796);
+
+static const struct rcar_du_of_overlay rcar_du_lvds_overlays[] __initconst = {
+	RCAR_DU_OF_OVERLAY(lvds, r8a7790),
+	RCAR_DU_OF_OVERLAY(lvds, r8a7791),
+	RCAR_DU_OF_OVERLAY(lvds, r8a7793),
+	RCAR_DU_OF_OVERLAY(lvds, r8a7795),
+	RCAR_DU_OF_OVERLAY(lvds, r8a7796),
+	{ /* Sentinel */ },
+};
+
+static struct of_changeset rcar_du_lvds_changeset;
+
+static void __init rcar_du_of_lvds_patch_one(struct device_node *lvds,
+					     const struct of_phandle_args *clk,
+					     struct device_node *local,
+					     struct device_node *remote)
+{
+	unsigned int psize;
+	unsigned int i;
+	__be32 value[4];
+	int ret;
+
+	/*
+	 * Set the LVDS clocks property. This can't be performed by the overlay
+	 * as the structure of the clock specifier has changed over time, and we
+	 * don't know at compile time which binding version the system we will
+	 * run on uses.
+	 */
+	if (clk->args_count >= ARRAY_SIZE(value) - 1)
+		return;
+
+	of_changeset_init(&rcar_du_lvds_changeset);
+
+	value[0] = cpu_to_be32(clk->np->phandle);
+	for (i = 0; i < clk->args_count; ++i)
+		value[i + 1] = cpu_to_be32(clk->args[i]);
+
+	psize = (clk->args_count + 1) * 4;
+	ret = of_changeset_add_property_copy(&rcar_du_lvds_changeset, lvds,
+					     "clocks", value, psize);
+	if (ret < 0)
+		goto done;
+
+	/*
+	 * Insert the node in the OF graph: patch the LVDS ports remote-endpoint
+	 * properties to point to the endpoints of the sibling nodes in the
+	 * graph. This can't be performed by the overlay: on the input side the
+	 * overlay would contain a phandle for the DU LVDS output port that
+	 * would clash with the system DT, and on the output side the connection
+	 * is board-specific.
+	 */
+	value[0] = cpu_to_be32(local->phandle);
+	value[1] = cpu_to_be32(remote->phandle);
+
+	for (i = 0; i < 2; ++i) {
+		struct device_node *endpoint;
+
+		endpoint = of_graph_get_endpoint_by_regs(lvds, i, 0);
+		if (!endpoint) {
+			ret = -EINVAL;
+			goto done;
+		}
+
+		ret = of_changeset_add_property_copy(&rcar_du_lvds_changeset,
+						endpoint, "remote-endpoint",
+						&value[i], sizeof(value[i]));
+		of_node_put(endpoint);
+		if (ret < 0)
+			goto done;
+	}
+
+	ret = of_changeset_apply(&rcar_du_lvds_changeset);
+
+done:
+	if (ret < 0)
+		of_changeset_destroy(&rcar_du_lvds_changeset);
+}
+
+struct lvds_of_data {
+	struct resource res;
+	struct of_phandle_args clkspec;
+	struct device_node *local;
+	struct device_node *remote;
+};
+
+static void __init rcar_du_of_lvds_patch(const struct of_device_id *of_ids)
+{
+	const struct rcar_du_device_info *info;
+	const struct of_device_id *match;
+	struct lvds_of_data lvds_data[2] = { };
+	struct device_node *lvds_node;
+	struct device_node *soc_node;
+	struct device_node *du_node;
+	char compatible[21];
+	const char *soc_name;
+	unsigned int i;
+	int ret;
+
+	/* Get the DU node and exit if not present or disabled. */
+	du_node = of_find_matching_node_and_match(NULL, of_ids, &match);
+	if (!du_node || !of_device_is_available(du_node)) {
+		of_node_put(du_node);
+		return;
+	}
+
+	info = match->data;
+	soc_node = of_get_parent(du_node);
+
+	if (WARN_ON(info->num_lvds > ARRAY_SIZE(lvds_data)))
+		goto done;
+
+	/*
+	 * Skip if the LVDS nodes already exists.
+	 *
+	 * The nodes are searched based on the compatible string, which we
+	 * construct from the SoC name found in the DU compatible string. As a
+	 * match has been found we know the compatible string matches the
+	 * expected format and can thus skip some of the string manipulation
+	 * normal safety checks.
+	 */
+	soc_name = strchr(match->compatible, '-') + 1;
+	sprintf(compatible, "renesas,%s-lvds", soc_name);
+	lvds_node = of_find_compatible_node(NULL, NULL, compatible);
+	if (lvds_node) {
+		of_node_put(lvds_node);
+		return;
+	}
+
+	/*
+	 * Parse the DU node and store the register specifier, the clock
+	 * specifier and the local and remote endpoint of the LVDS link for
+	 * later use.
+	 */
+	for (i = 0; i < info->num_lvds; ++i) {
+		struct lvds_of_data *lvds = &lvds_data[i];
+		unsigned int port;
+		char name[7];
+		int index;
+
+		sprintf(name, "lvds.%u", i);
+		index = of_property_match_string(du_node, "clock-names", name);
+		if (index < 0)
+			continue;
+
+		ret = of_parse_phandle_with_args(du_node, "clocks",
+						 "#clock-cells", index,
+						 &lvds->clkspec);
+		if (ret < 0)
+			continue;
+
+		port = info->routes[RCAR_DU_OUTPUT_LVDS0 + i].port;
+
+		lvds->local = of_graph_get_endpoint_by_regs(du_node, port, 0);
+		if (!lvds->local)
+			continue;
+
+		lvds->remote = of_graph_get_remote_endpoint(lvds->local);
+		if (!lvds->remote)
+			continue;
+
+		index = of_property_match_string(du_node, "reg-names", name);
+		if (index < 0)
+			continue;
+
+		of_address_to_resource(du_node, index, &lvds->res);
+	}
+
+	/* Parse and apply the overlay. This will resolve phandles. */
+	ret = rcar_du_of_apply_overlay(rcar_du_lvds_overlays,
+				       match->compatible);
+	if (ret < 0)
+		goto done;
+
+	/* Patch the newly created LVDS encoder nodes. */
+	for_each_child_of_node(soc_node, lvds_node) {
+		struct resource res;
+
+		if (!of_device_is_compatible(lvds_node, compatible))
+			continue;
+
+		/* Locate the lvds_data entry based on the resource start. */
+		ret = of_address_to_resource(lvds_node, 0, &res);
+		if (ret < 0)
+			continue;
+
+		for (i = 0; i < ARRAY_SIZE(lvds_data); ++i) {
+			if (lvds_data[i].res.start == res.start)
+				break;
+		}
+
+		if (i == ARRAY_SIZE(lvds_data))
+			continue;
+
+		/* Patch the LVDS encoder. */
+		rcar_du_of_lvds_patch_one(lvds_node, &lvds_data[i].clkspec,
+					  lvds_data[i].local,
+					  lvds_data[i].remote);
+	}
+
+done:
+	for (i = 0; i < info->num_lvds; ++i) {
+		of_node_put(lvds_data[i].clkspec.np);
+		of_node_put(lvds_data[i].local);
+		of_node_put(lvds_data[i].remote);
+	}
+
+	of_node_put(soc_node);
+	of_node_put(du_node);
+}
+
+void __init rcar_du_of_init(const struct of_device_id *of_ids)
+{
+	rcar_du_of_lvds_patch(of_ids);
+}
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of.h b/drivers/gpu/drm/rcar-du/rcar_du_of.h
new file mode 100644
index 000000000000..c2e65a727e91
--- /dev/null
+++ b/drivers/gpu/drm/rcar-du/rcar_du_of.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * rcar_du_of.h - Legacy DT bindings compatibility
+ *
+ * Copyright (C) 2018 Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+ */
+#ifndef __RCAR_DU_OF_H__
+#define __RCAR_DU_OF_H__
+
+#include <linux/init.h>
+
+struct of_device_id;
+
+#ifdef CONFIG_DRM_RCAR_LVDS
+void __init rcar_du_of_init(const struct of_device_id *of_ids);
+#else
+static inline void rcar_du_of_init(const struct of_device_id *of_ids) { }
+#endif /* CONFIG_DRM_RCAR_LVDS */
+
+#endif /* __RCAR_DU_OF_H__ */
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 000000000000..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 <laurent.pinchart@ideasonboard.com>
+ *
+ * Based on work from Jyri Sarha <jsarha@ti.com>
+ * Copyright (C) 2015 Texas Instruments
+ */
+
+#include <dt-bindings/clock/renesas-cpg-mssr.h>
+
+/dts-v1/;
+/plugin/;
+/ {
+	fragment@0 {
+		target-path = "/";
+		__overlay__ {
+			#address-cells = <2>;
+			#size-cells = <2>;
+
+			lvds@feb90000 {
+				compatible = "renesas,r8a7790-lvds";
+				reg = <0 0xfeb90000 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@feb00000/ports";
+		__overlay__ {
+			port@1 {
+				endpoint {
+					remote-endpoint = <&lvds0_input>;
+				};
+			};
+			port@2 {
+				endpoint {
+					remote-endpoint = <&lvds1_input>;
+				};
+			};
+		};
+	};
+};
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts b/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts
new file mode 100644
index 000000000000..601982bba27d
--- /dev/null
+++ b/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * rcar_du_of_lvds_r8a7791.dts - Legacy LVDS DT bindings conversion for R8A7791
+ *
+ * Copyright (C) 2018 Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+ *
+ * Based on work from Jyri Sarha <jsarha@ti.com>
+ * Copyright (C) 2015 Texas Instruments
+ */
+
+#include <dt-bindings/clock/renesas-cpg-mssr.h>
+
+/dts-v1/;
+/plugin/;
+/ {
+	fragment@0 {
+		target-path = "/";
+		__overlay__ {
+			#address-cells = <2>;
+			#size-cells = <2>;
+
+			lvds@feb90000 {
+				compatible = "renesas,r8a7791-lvds";
+				reg = <0 0xfeb90000 0 0x1c>;
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+						lvds0_input: endpoint {
+						};
+					};
+					port@1 {
+						reg = <1>;
+						lvds0_out: endpoint {
+						};
+					};
+				};
+			};
+		};
+	};
+
+	fragment@1 {
+		target-path = "/display@feb00000/ports";
+		__overlay__ {
+			port@1 {
+				endpoint {
+					remote-endpoint = <&lvds0_input>;
+				};
+			};
+		};
+	};
+};
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts b/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts
new file mode 100644
index 000000000000..1c535954a529
--- /dev/null
+++ b/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * rcar_du_of_lvds_r8a7793.dts - Legacy LVDS DT bindings conversion for R8A7793
+ *
+ * Copyright (C) 2018 Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+ *
+ * Based on work from Jyri Sarha <jsarha@ti.com>
+ * Copyright (C) 2015 Texas Instruments
+ */
+
+#include <dt-bindings/clock/renesas-cpg-mssr.h>
+
+/dts-v1/;
+/plugin/;
+/ {
+	fragment@0 {
+		target-path = "/";
+		__overlay__ {
+			#address-cells = <2>;
+			#size-cells = <2>;
+
+			lvds@feb90000 {
+				compatible = "renesas,r8a7793-lvds";
+				reg = <0 0xfeb90000 0 0x1c>;
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+						lvds0_input: endpoint {
+						};
+					};
+					port@1 {
+						reg = <1>;
+						lvds0_out: endpoint {
+						};
+					};
+				};
+			};
+		};
+	};
+
+	fragment@1 {
+		target-path = "/display@feb00000/ports";
+		__overlay__ {
+			port@1 {
+				endpoint {
+					remote-endpoint = <&lvds0_input>;
+				};
+			};
+		};
+	};
+};
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts b/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts
new file mode 100644
index 000000000000..44e52101ef05
--- /dev/null
+++ b/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * rcar_du_of_lvds_r8a7795.dts - Legacy LVDS DT bindings conversion for R8A7795
+ *
+ * Copyright (C) 2018 Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+ *
+ * Based on work from Jyri Sarha <jsarha@ti.com>
+ * Copyright (C) 2015 Texas Instruments
+ */
+
+#include <dt-bindings/clock/renesas-cpg-mssr.h>
+
+/dts-v1/;
+/plugin/;
+/ {
+	fragment@0 {
+		target-path = "/soc";
+		__overlay__ {
+			#address-cells = <2>;
+			#size-cells = <2>;
+
+			lvds@feb90000 {
+				compatible = "renesas,r8a7795-lvds";
+				reg = <0 0xfeb90000 0 0x14>;
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+						lvds0_input: endpoint {
+						};
+					};
+					port@1 {
+						reg = <1>;
+						lvds0_out: endpoint {
+						};
+					};
+				};
+			};
+		};
+	};
+
+	fragment@1 {
+		target-path = "/soc/display@feb00000/ports";
+		__overlay__ {
+			port@3 {
+				endpoint {
+					remote-endpoint = <&lvds0_input>;
+				};
+			};
+		};
+	};
+};
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts b/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts
new file mode 100644
index 000000000000..8f45ebe140bd
--- /dev/null
+++ b/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * rcar_du_of_lvds_r8a7796.dts - Legacy LVDS DT bindings conversion for R8A7796
+ *
+ * Copyright (C) 2018 Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+ *
+ * Based on work from Jyri Sarha <jsarha@ti.com>
+ * Copyright (C) 2015 Texas Instruments
+ */
+
+#include <dt-bindings/clock/renesas-cpg-mssr.h>
+
+/dts-v1/;
+/plugin/;
+/ {
+	fragment@0 {
+		target-path = "/soc";
+		__overlay__ {
+			#address-cells = <2>;
+			#size-cells = <2>;
+
+			lvds@feb90000 {
+				compatible = "renesas,r8a7796-lvds";
+				reg = <0 0xfeb90000 0 0x14>;
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+						lvds0_input: endpoint {
+						};
+					};
+					port@1 {
+						reg = <1>;
+						lvds0_out: endpoint {
+						};
+					};
+				};
+			};
+		};
+	};
+
+	fragment@1 {
+		target-path = "/soc/display@feb00000/ports";
+		__overlay__ {
+			port@3 {
+				endpoint {
+					remote-endpoint = <&lvds0_input>;
+				};
+			};
+		};
+	};
+};
-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 07/16] i2c: demux: Use changeset helpers for clarity
  2018-02-20 23:10 ` [PATCH v4 07/16] i2c: demux: Use changeset helpers for clarity Laurent Pinchart
@ 2018-02-21  8:06   ` Wolfram Sang
  0 siblings, 0 replies; 33+ messages in thread
From: Wolfram Sang @ 2018-02-21  8:06 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Marek Vasut, devicetree, Koen Kooi, Pantelis Antoniou,
	linux-kernel, dri-devel, linux-renesas-soc, linux-i2c,
	Matt Porter, Frank Rowand, Guenter Roeck


[-- Attachment #1.1: Type: text/plain, Size: 400 bytes --]

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

My ack still holds. Good luck pushing this series!


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 01/16] dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings
  2018-02-20 23:10 ` [PATCH v4 01/16] dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings Laurent Pinchart
@ 2018-02-21  8:35   ` Sergei Shtylyov
  2018-02-21 19:50     ` Laurent Pinchart
  0 siblings, 1 reply; 33+ messages in thread
From: Sergei Shtylyov @ 2018-02-21  8:35 UTC (permalink / raw)
  To: Laurent Pinchart, dri-devel; +Cc: linux-renesas-soc, devicetree

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 <laurent.pinchart+renesas@ideasonboard.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
> 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 000000000000..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@feb90000 {

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

[...]

MBR, Sergei
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 04/16] of: changesets: Introduce changeset helper methods
  2018-02-20 23:10 ` [PATCH v4 04/16] of: changesets: Introduce changeset helper methods Laurent Pinchart
@ 2018-02-21 10:21   ` Geert Uytterhoeven
  2018-02-21 12:19     ` Laurent Pinchart
  2018-02-21 15:23     ` Rob Herring
  2018-02-21 23:14   ` Rob Herring
  1 sibling, 2 replies; 33+ messages in thread
From: Geert Uytterhoeven @ 2018-02-21 10:21 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Marek Vasut, Wolfram Sang, Koen Kooi, Pantelis Antoniou,
	Linux Kernel Mailing List, DRI Development, Linux-Renesas,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux I2C, Matt Porter, Frank Rowand, Guenter Roeck

Hi Laurent,

On Wed, Feb 21, 2018 at 12:10 AM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> From: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
>
> 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 <pantelis.antoniou@konsulko.com>
> [Fixed memory leak in __of_changeset_add_update_property_copy()]
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

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&id=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, &val,
> +                       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, &val,
> +                       sizeof(val), true);
> +}

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 03/16] of: dynamic: Add __of_node_dupv()
  2018-02-20 23:10 ` [PATCH v4 03/16] of: dynamic: Add __of_node_dupv() Laurent Pinchart
@ 2018-02-21 10:26   ` Geert Uytterhoeven
  2018-02-21 12:15     ` Laurent Pinchart
  2018-02-21 23:16   ` Rob Herring
  1 sibling, 1 reply; 33+ messages in thread
From: Geert Uytterhoeven @ 2018-02-21 10:26 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Marek Vasut, Wolfram Sang, Koen Kooi, Pantelis Antoniou,
	Linux Kernel Mailing List, DRI Development, Linux-Renesas,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux I2C, Matt Porter, Frank Rowand, Guenter Roeck

Hi Laurent,

On Wed, Feb 21, 2018 at 12:10 AM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> From: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
>
> 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 <pantelis.antoniou@konsulko.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  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&id=c45324e1807dd708344c9a478b777b68aca11cdf

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 03/16] of: dynamic: Add __of_node_dupv()
  2018-02-21 10:26   ` Geert Uytterhoeven
@ 2018-02-21 12:15     ` Laurent Pinchart
  0 siblings, 0 replies; 33+ messages in thread
From: Laurent Pinchart @ 2018-02-21 12:15 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Marek Vasut, Laurent Pinchart, Wolfram Sang, Koen Kooi,
	Pantelis Antoniou, Linux Kernel Mailing List, DRI Development,
	Linux-Renesas,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux I2C, Matt Porter, Frank Rowand, Guenter Roeck

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 <pantelis.antoniou@konsulko.com>
> > 
> > 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 <pantelis.antoniou@konsulko.com>
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >  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&id=c45324e1807dd708344c9a478b777b68aca11cdf

I'll fix that in the next version.

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 04/16] of: changesets: Introduce changeset helper methods
  2018-02-21 10:21   ` Geert Uytterhoeven
@ 2018-02-21 12:19     ` Laurent Pinchart
  2018-02-21 15:23     ` Rob Herring
  1 sibling, 0 replies; 33+ messages in thread
From: Laurent Pinchart @ 2018-02-21 12:19 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Marek Vasut, Laurent Pinchart, Wolfram Sang, Koen Kooi,
	Pantelis Antoniou, Linux Kernel Mailing List, DRI Development,
	Linux-Renesas,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux I2C, Matt Porter, Frank Rowand, Guenter Roeck

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 <pantelis.antoniou@konsulko.com>
> > 
> > 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 <pantelis.antoniou@konsulko.com>
> > [Fixed memory leak in __of_changeset_add_update_property_copy()]
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> 
> 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&id=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,
> > &val,
> > +                       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,
> > &val,
> > +                       sizeof(val), true);
> > +}

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 04/16] of: changesets: Introduce changeset helper methods
  2018-02-21 10:21   ` Geert Uytterhoeven
  2018-02-21 12:19     ` Laurent Pinchart
@ 2018-02-21 15:23     ` Rob Herring
  2018-02-21 15:46       ` Geert Uytterhoeven
  1 sibling, 1 reply; 33+ messages in thread
From: Rob Herring @ 2018-02-21 15:23 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Marek Vasut,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Laurent Pinchart, Wolfram Sang, Koen Kooi, Pantelis Antoniou,
	Linux Kernel Mailing List, DRI Development, Linux-Renesas,
	Linux I2C, Matt Porter, Frank Rowand, Guenter Roeck

On Wed, Feb 21, 2018 at 4:21 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Laurent,
>
> On Wed, Feb 21, 2018 at 12:10 AM, Laurent Pinchart
> <laurent.pinchart+renesas@ideasonboard.com> wrote:
>> From: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
>>
>> 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 <pantelis.antoniou@konsulko.com>
>> [Fixed memory leak in __of_changeset_add_update_property_copy()]
>> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>
> 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&id=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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 04/16] of: changesets: Introduce changeset helper methods
  2018-02-21 15:23     ` Rob Herring
@ 2018-02-21 15:46       ` Geert Uytterhoeven
  0 siblings, 0 replies; 33+ messages in thread
From: Geert Uytterhoeven @ 2018-02-21 15:46 UTC (permalink / raw)
  To: Rob Herring
  Cc: Marek Vasut,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Laurent Pinchart, Wolfram Sang, Koen Kooi, Pantelis Antoniou,
	Linux Kernel Mailing List, DRI Development, Linux-Renesas,
	Linux I2C, Matt Porter, Frank Rowand, Guenter Roeck

Hi Rob,

On Wed, Feb 21, 2018 at 4:23 PM, Rob Herring <robh@kernel.org> wrote:
> On Wed, Feb 21, 2018 at 4:21 AM, Geert Uytterhoeven
> <geert@linux-m68k.org> 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&id=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 -- geert@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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 00/16] R-Car DU: Convert LVDS code to bridge driver
  2018-02-20 23:10 [PATCH v4 00/16] R-Car DU: Convert LVDS code to bridge driver Laurent Pinchart
                   ` (7 preceding siblings ...)
  2018-02-20 23:10 ` [PATCH v4 08/16] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes Laurent Pinchart
@ 2018-02-21 16:39 ` Simon Horman
  2018-02-22  6:07 ` Frank Rowand
  9 siblings, 0 replies; 33+ messages in thread
From: Simon Horman @ 2018-02-21 16:39 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Marek Vasut, Wolfram Sang, Koen Kooi, Pantelis Antoniou,
	linux-kernel, dri-devel, linux-renesas-soc, devicetree,
	linux-i2c, Matt Porter, Frank Rowand, Guenter Roeck

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

...
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 01/16] dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings
  2018-02-21  8:35   ` Sergei Shtylyov
@ 2018-02-21 19:50     ` Laurent Pinchart
  0 siblings, 0 replies; 33+ messages in thread
From: Laurent Pinchart @ 2018-02-21 19:50 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: linux-renesas-soc, devicetree, Laurent Pinchart, dri-devel

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
> > <laurent.pinchart+renesas@ideasonboard.com>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > ---
> > 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 000000000000..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@feb90000 {
> 
> "lvds-encoder@feb90000" maybe?

Or just "encoder@feb90000" ? Or "display-encoder@feb90000" ? 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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 06/16] of: unittest: changeset helpers
  2018-02-20 23:10 ` [PATCH v4 06/16] of: unittest: changeset helpers Laurent Pinchart
@ 2018-02-21 23:10   ` Rob Herring
  2018-02-21 23:39     ` Laurent Pinchart
  0 siblings, 1 reply; 33+ messages in thread
From: Rob Herring @ 2018-02-21 23:10 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Marek Vasut, devicetree, Wolfram Sang, Koen Kooi,
	Pantelis Antoniou, linux-kernel, dri-devel,
	open list:MEDIA DRIVERS FOR RENESAS - FCP, Linux I2C,
	Matt Porter, Frank Rowand, Guenter Roeck

On Tue, Feb 20, 2018 at 5:10 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> From: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
>
> Add a unitest specific for the new changeset helpers.
>
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  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 <robh@kernel.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 04/16] of: changesets: Introduce changeset helper methods
  2018-02-20 23:10 ` [PATCH v4 04/16] of: changesets: Introduce changeset helper methods Laurent Pinchart
  2018-02-21 10:21   ` Geert Uytterhoeven
@ 2018-02-21 23:14   ` Rob Herring
  1 sibling, 0 replies; 33+ messages in thread
From: Rob Herring @ 2018-02-21 23:14 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Marek Vasut, devicetree, Wolfram Sang, Koen Kooi,
	Pantelis Antoniou, linux-kernel, dri-devel,
	open list:MEDIA DRIVERS FOR RENESAS - FCP, Linux I2C,
	Matt Porter, Frank Rowand, Guenter Roeck

On Tue, Feb 20, 2018 at 5:10 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> From: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
>
> 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 <pantelis.antoniou@konsulko.com>
> [Fixed memory leak in __of_changeset_add_update_property_copy()]
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  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 <robh@kernel.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 03/16] of: dynamic: Add __of_node_dupv()
  2018-02-20 23:10 ` [PATCH v4 03/16] of: dynamic: Add __of_node_dupv() Laurent Pinchart
  2018-02-21 10:26   ` Geert Uytterhoeven
@ 2018-02-21 23:16   ` Rob Herring
  1 sibling, 0 replies; 33+ messages in thread
From: Rob Herring @ 2018-02-21 23:16 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Marek Vasut, devicetree, Wolfram Sang, Koen Kooi,
	Pantelis Antoniou, linux-kernel, dri-devel,
	open list:MEDIA DRIVERS FOR RENESAS - FCP, Linux I2C,
	Matt Porter, Frank Rowand, Guenter Roeck

On Tue, Feb 20, 2018 at 5:10 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> From: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
>
> 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 <pantelis.antoniou@konsulko.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/of/dynamic.c | 29 +++++++++++++++++++++++------
>  1 file changed, 23 insertions(+), 6 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 05/16] of: changeset: Add of_changeset_node_move method
  2018-02-20 23:10 ` [PATCH v4 05/16] of: changeset: Add of_changeset_node_move method Laurent Pinchart
@ 2018-02-21 23:20   ` Rob Herring
  2018-02-21 23:40     ` Laurent Pinchart
  0 siblings, 1 reply; 33+ messages in thread
From: Rob Herring @ 2018-02-21 23:20 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Marek Vasut, devicetree, Wolfram Sang, Koen Kooi,
	Pantelis Antoniou, linux-kernel, dri-devel,
	open list:MEDIA DRIVERS FOR RENESAS - FCP, Linux I2C,
	Matt Porter, Frank Rowand, Guenter Roeck

On Tue, Feb 20, 2018 at 5:10 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> From: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
>
> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 08/16] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes
  2018-02-20 23:10 ` [PATCH v4 08/16] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes Laurent Pinchart
@ 2018-02-21 23:28   ` Rob Herring
  2018-02-21 23:54     ` Laurent Pinchart
  0 siblings, 1 reply; 33+ messages in thread
From: Rob Herring @ 2018-02-21 23:28 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: open list:MEDIA DRIVERS FOR RENESAS - FCP, devicetree,
	Frank Rowand, dri-devel

On Tue, Feb 20, 2018 at 5:10 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> 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 <laurent.pinchart+renesas@ideasonboard.com>
> ---

[...]

> 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 000000000000..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 <laurent.pinchart@ideasonboard.com>
> + *
> + * Based on work from Jyri Sarha <jsarha@ti.com>
> + * Copyright (C) 2015 Texas Instruments
> + */
> +
> +#include <dt-bindings/clock/renesas-cpg-mssr.h>

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

Otherwise,

Reviewed-by: Rob Herring <robh@kernel.org>

> +
> +/dts-v1/;
> +/plugin/;
> +/ {
> +       fragment@0 {
> +               target-path = "/";
> +               __overlay__ {
> +                       #address-cells = <2>;
> +                       #size-cells = <2>;
> +
> +                       lvds@feb90000 {
> +                               compatible = "renesas,r8a7790-lvds";
> +                               reg = <0 0xfeb90000 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@feb00000/ports";
> +               __overlay__ {
> +                       port@1 {
> +                               endpoint {
> +                                       remote-endpoint = <&lvds0_input>;
> +                               };
> +                       };
> +                       port@2 {
> +                               endpoint {
> +                                       remote-endpoint = <&lvds1_input>;
> +                               };
> +                       };
> +               };
> +       };
> +};
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 06/16] of: unittest: changeset helpers
  2018-02-21 23:10   ` Rob Herring
@ 2018-02-21 23:39     ` Laurent Pinchart
  2018-02-21 23:49       ` Rob Herring
  0 siblings, 1 reply; 33+ messages in thread
From: Laurent Pinchart @ 2018-02-21 23:39 UTC (permalink / raw)
  To: Rob Herring
  Cc: Marek Vasut, devicetree, Laurent Pinchart, Wolfram Sang,
	Koen Kooi, Pantelis Antoniou, linux-kernel, dri-devel,
	open list:MEDIA DRIVERS FOR RENESAS - FCP, Linux I2C,
	Matt Porter, Frank Rowand, Guenter Roeck

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 <pantelis.antoniou@konsulko.com>
> > 
> > Add a unitest specific for the new changeset helpers.
> > 
> > Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >  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 <robh@kernel.org>

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 05/16] of: changeset: Add of_changeset_node_move method
  2018-02-21 23:20   ` Rob Herring
@ 2018-02-21 23:40     ` Laurent Pinchart
  0 siblings, 0 replies; 33+ messages in thread
From: Laurent Pinchart @ 2018-02-21 23:40 UTC (permalink / raw)
  To: Rob Herring
  Cc: Marek Vasut, devicetree, Laurent Pinchart, Wolfram Sang,
	Koen Kooi, Pantelis Antoniou, linux-kernel, dri-devel,
	open list:MEDIA DRIVERS FOR RENESAS - FCP, Linux I2C,
	Matt Porter, Frank Rowand, Guenter Roeck

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 <pantelis.antoniou@konsulko.com>
> > 
> > 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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 06/16] of: unittest: changeset helpers
  2018-02-21 23:39     ` Laurent Pinchart
@ 2018-02-21 23:49       ` Rob Herring
  0 siblings, 0 replies; 33+ messages in thread
From: Rob Herring @ 2018-02-21 23:49 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Marek Vasut, devicetree, Laurent Pinchart, Wolfram Sang,
	Koen Kooi, Pantelis Antoniou, linux-kernel, dri-devel,
	open list:MEDIA DRIVERS FOR RENESAS - FCP, Linux I2C,
	Matt Porter, Frank Rowand, Guenter Roeck

On Wed, Feb 21, 2018 at 5:39 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> 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 <pantelis.antoniou@konsulko.com>
>> >
>> > Add a unitest specific for the new changeset helpers.
>> >
>> > Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
>> > Signed-off-by: Laurent Pinchart
>> > <laurent.pinchart+renesas@ideasonboard.com>
>> > ---
>> >
>> >  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 <robh@kernel.org>
>
> --
> Regards,
>
> Laurent Pinchart
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 08/16] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes
  2018-02-21 23:28   ` Rob Herring
@ 2018-02-21 23:54     ` Laurent Pinchart
  0 siblings, 0 replies; 33+ messages in thread
From: Laurent Pinchart @ 2018-02-21 23:54 UTC (permalink / raw)
  To: Rob Herring
  Cc: open list:MEDIA DRIVERS FOR RENESAS - FCP, devicetree,
	Laurent Pinchart, Frank Rowand, dri-devel

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
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> 
> [...]
> 
> > 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 000000000000..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
> > <laurent.pinchart@ideasonboard.com>
> > + *
> > + * Based on work from Jyri Sarha <jsarha@ti.com>
> > + * Copyright (C) 2015 Texas Instruments
> > + */
> > +
> > +#include <dt-bindings/clock/renesas-cpg-mssr.h>
> 
> 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 <robh@kernel.org>
> 
> > +
> > +/dts-v1/;
> > +/plugin/;
> > +/ {
> > +       fragment@0 {
> > +               target-path = "/";
> > +               __overlay__ {
> > +                       #address-cells = <2>;
> > +                       #size-cells = <2>;
> > +
> > +                       lvds@feb90000 {
> > +                               compatible = "renesas,r8a7790-lvds";
> > +                               reg = <0 0xfeb90000 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@feb00000/ports";
> > +               __overlay__ {
> > +                       port@1 {
> > +                               endpoint {
> > +                                       remote-endpoint = <&lvds0_input>;
> > +                               };
> > +                       };
> > +                       port@2 {
> > +                               endpoint {
> > +                                       remote-endpoint = <&lvds1_input>;
> > +                               };
> > +                       };
> > +               };
> > +       };
> > +};

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 00/16] R-Car DU: Convert LVDS code to bridge driver
  2018-02-20 23:10 [PATCH v4 00/16] R-Car DU: Convert LVDS code to bridge driver Laurent Pinchart
                   ` (8 preceding siblings ...)
  2018-02-21 16:39 ` [PATCH v4 00/16] R-Car DU: Convert LVDS code to bridge driver Simon Horman
@ 2018-02-22  6:07 ` Frank Rowand
  2018-02-22 10:25   ` Laurent Pinchart
  9 siblings, 1 reply; 33+ messages in thread
From: Frank Rowand @ 2018-02-22  6:07 UTC (permalink / raw)
  To: Laurent Pinchart, dri-devel
  Cc: Marek Vasut, Wolfram Sang, Koen Kooi, Pantelis Antoniou,
	linux-kernel, linux-renesas-soc, devicetree, linux-i2c,
	Matt Porter, Guenter Roeck

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 +-
>  arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts     |   3 +-
>  arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts |   3 +-
>  .../arm64/boot/dts/renesas/r8a7795-salvator-xs.dts |   3 +-
>  arch/arm64/boot/dts/renesas/r8a7795.dtsi           |  36 +-
>  arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts     |   3 +-
>  arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts |   3 +-
>  arch/arm64/boot/dts/renesas/r8a7796.dtsi           |  36 +-
>  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    |  81 ++++
>  .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts    |  55 +++
>  .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts    |  55 +++
>  .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts    |  55 +++
>  .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts    |  55 +++
>  drivers/gpu/drm/rcar-du/rcar_lvds.c                | 524 +++++++++++++++++++++
>  drivers/i2c/muxes/i2c-demux-pinctrl.c              |  12 +-
>  drivers/of/dynamic.c                               | 317 ++++++++++++-
>  drivers/of/unittest.c                              |  54 +++
>  include/linux/of.h                                 | 337 +++++++++++++
>  43 files changed, 2199 insertions(+), 710 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
>  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_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
>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_lvds.c
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 00/16] R-Car DU: Convert LVDS code to bridge driver
  2018-02-22  6:07 ` Frank Rowand
@ 2018-02-22 10:25   ` Laurent Pinchart
  2018-02-23  3:20     ` Frank Rowand
  0 siblings, 1 reply; 33+ messages in thread
From: Laurent Pinchart @ 2018-02-22 10:25 UTC (permalink / raw)
  To: Frank Rowand
  Cc: Marek Vasut, Laurent Pinchart, Wolfram Sang, Koen Kooi,
	Pantelis Antoniou, linux-kernel, dri-devel, linux-renesas-soc,
	devicetree, linux-i2c, Matt Porter, Guenter Roeck

Hi Frank,

On Thursday, 22 February 2018 08:07:14 EET Frank Rowand wrote:
> 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.

Because Rob asked for the driver-local implementation of the property add 
function to be replaced by Pantelis' series. I want to get the LVDS changes in 
v4.17 and asked Rob whether I could then take the OF changeset patches merged 
through the DRM tree, and he didn't object. If that causes an issue I'll 
switch back to the driver-local implementation to get the driver changes 
merged, split the OF changeset series out, and then move to the OF changeset 
API once merged. Would you prefer that ?

> 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.
> 
> > 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 +-
> >  arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts     |   3 +-
> >  arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts |   3 +-
> >  .../arm64/boot/dts/renesas/r8a7795-salvator-xs.dts |   3 +-
> >  arch/arm64/boot/dts/renesas/r8a7795.dtsi           |  36 +-
> >  arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts     |   3 +-
> >  arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts |   3 +-
> >  arch/arm64/boot/dts/renesas/r8a7796.dtsi           |  36 +-
> >  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    |  81 ++++
> >  .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts    |  55 +++
> >  .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts    |  55 +++
> >  .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts    |  55 +++
> >  .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts    |  55 +++
> >  drivers/gpu/drm/rcar-du/rcar_lvds.c                | 524 ++++++++++++++++
> >  drivers/i2c/muxes/i2c-demux-pinctrl.c              |  12 +-
> >  drivers/of/dynamic.c                               | 317 ++++++++++++-
> >  drivers/of/unittest.c                              |  54 +++
> >  include/linux/of.h                                 | 337 +++++++++++++
> >  43 files changed, 2199 insertions(+), 710 deletions(-)
> >  create mode 100644
> >  Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt 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_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
> >  create mode 100644 drivers/gpu/drm/rcar-du/rcar_lvds.c

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 00/16] R-Car DU: Convert LVDS code to bridge driver
  2018-02-22 10:25   ` Laurent Pinchart
@ 2018-02-23  3:20     ` Frank Rowand
  2018-02-23  9:25       ` Laurent Pinchart
  0 siblings, 1 reply; 33+ messages in thread
From: Frank Rowand @ 2018-02-23  3:20 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Marek Vasut, Laurent Pinchart, Wolfram Sang, Koen Kooi,
	Pantelis Antoniou, linux-kernel, dri-devel, linux-renesas-soc,
	devicetree, linux-i2c, Matt Porter, Guenter Roeck

Hi Laurent,

On 02/22/18 02:25, Laurent Pinchart wrote:
> Hi Frank,
> 
> On Thursday, 22 February 2018 08:07:14 EET Frank Rowand wrote:
>> 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.
> 
> Because Rob asked for the driver-local implementation of the property add 
> function to be replaced by Pantelis' series. I want to get the LVDS changes in 
> v4.17 and asked Rob whether I could then take the OF changeset patches merged 
> through the DRM tree, and he didn't object. If that causes an issue I'll 
> switch back to the driver-local implementation to get the driver changes 
> merged, split the OF changeset series out, and then move to the OF changeset 
> API once merged. Would you prefer that ?

You have already created a new version of the R-Car patches without the
set of patches that I was objecting to here.  So this is somewhat just
an academic comment.

As I mentioned in the v6 thread, I am coming back here to clean up loose
ends, and explain why I had the reaction I had.  Basically, this is a
process issue to me.

(1) The patch set from Pantelis is "hidden" in the driver patch series.
When viewing collapsed threads (which is my normal mode to avoid getting
overwhelmed by the vast volume of email I scan), the Pantelis patch set
is totally invisible.  If the R-Car driver patch series had not had me
on the to: list, there is a very good chance I would not have noticed
it.  Or noticed in a more delayed time frame.  And the same applies to
anyone else who might be subscribed to the devicetree mail list.  If
the Pantelis patch series was split out into a separate patch set then
it would be more visible on the list.

(2) There is no good way to indicate in the email subject lines for
the Pantelis patches that they are version 3 of the series, since
they are also version 4 of the R-Car patch series.  If one reads the
patch 0/0 header carefully, and/or the other Pantelis patch comment
headers carefully, and then does a little detective work, it is
possible to find version 2 of the Pantelis patch series, and thus
be able to track the full history.  If just glancing at each
individual patch email subject, scanning the patch comment, and
spending more time reading the body of the patch, it would be
very easy to overlook the existence of the previous versions on
the mail list.

(2b) Small quibble:  if the Pantelis patches were in a separate series,
with v3 in the subject header, then you would have normally put
a "changes since v2" section in the patch comment header, which would
have been more visible and less cryptic than what you wrote, which was:

   ...

   Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
   [Fixed memory leak in __of_changeset_add_update_property_copy()]
   Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
   ---
    drivers/of/dynamic.c | 222 ++++++++++++++++++++++++++++++++++
    include/linux/of.h   | 328 +++++++++++++++++++++++++++++++++++++++++++++++++++
    2 files changed, 550 insertions(+)

My original scan of version 2 and then the email in this series had me
questioning whether the two open issues from patch 2 had been addressed
(yes, patch 0/16 did explicitly mention that 2 review comments had been
taken into account, so you can point at my poor reading comprehension).

(3) This is totally unrelated to your patch series, but I'm leaving a bread
crumb trail here.  Rob pointed you at a patch series that was not the most
recent version.  When this patch series appears again, there already is
a version 3 subset of it, but it is still labeled as being version 2, and
also has outstanding un-addressed issues.  ("[PATCH v2 1/2] of: dynamic:
Add __of_node_dupv()", 11/04/16)

-Frank
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 00/16] R-Car DU: Convert LVDS code to bridge driver
  2018-02-23  3:20     ` Frank Rowand
@ 2018-02-23  9:25       ` Laurent Pinchart
  2018-02-23 19:35         ` Frank Rowand
  0 siblings, 1 reply; 33+ messages in thread
From: Laurent Pinchart @ 2018-02-23  9:25 UTC (permalink / raw)
  To: Frank Rowand
  Cc: Marek Vasut, Laurent Pinchart, Wolfram Sang, Koen Kooi,
	Pantelis Antoniou, linux-kernel, dri-devel, linux-renesas-soc,
	devicetree, linux-i2c, Matt Porter, Guenter Roeck

Hi Frank,

On Friday, 23 February 2018 05:20:43 EET Frank Rowand wrote:
> On 02/22/18 02:25, Laurent Pinchart wrote:
> > On Thursday, 22 February 2018 08:07:14 EET Frank Rowand wrote:
> >> 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.
> > 
> > Because Rob asked for the driver-local implementation of the property add
> > function to be replaced by Pantelis' series. I want to get the LVDS
> > changes in v4.17 and asked Rob whether I could then take the OF changeset
> > patches merged through the DRM tree, and he didn't object. If that causes
> > an issue I'll switch back to the driver-local implementation to get the
> > driver changes merged, split the OF changeset series out, and then move
> > to the OF changeset API once merged. Would you prefer that ?
> 
> You have already created a new version of the R-Car patches without the
> set of patches that I was objecting to here.  So this is somewhat just
> an academic comment.
> 
> As I mentioned in the v6 thread, I am coming back here to clean up loose
> ends, and explain why I had the reaction I had.  Basically, this is a
> process issue to me.
> 
> (1) The patch set from Pantelis is "hidden" in the driver patch series.
> When viewing collapsed threads (which is my normal mode to avoid getting
> overwhelmed by the vast volume of email I scan), the Pantelis patch set
> is totally invisible.  If the R-Car driver patch series had not had me
> on the to: list, there is a very good chance I would not have noticed
> it.  Or noticed in a more delayed time frame.  And the same applies to
> anyone else who might be subscribed to the devicetree mail list.  If
> the Pantelis patch series was split out into a separate patch set then
> it would be more visible on the list.
> 
> (2) There is no good way to indicate in the email subject lines for
> the Pantelis patches that they are version 3 of the series, since
> they are also version 4 of the R-Car patch series.  If one reads the
> patch 0/0 header carefully, and/or the other Pantelis patch comment
> headers carefully, and then does a little detective work, it is
> possible to find version 2 of the Pantelis patch series, and thus
> be able to track the full history.  If just glancing at each
> individual patch email subject, scanning the patch comment, and
> spending more time reading the body of the patch, it would be
> very easy to overlook the existence of the previous versions on
> the mail list.
> 
> (2b) Small quibble:  if the Pantelis patches were in a separate series,
> with v3 in the subject header, then you would have normally put
> a "changes since v2" section in the patch comment header, which would
> have been more visible and less cryptic than what you wrote, which was:
> 
>    ...
> 
>    Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
>    [Fixed memory leak in __of_changeset_add_update_property_copy()]
>    Signed-off-by: Laurent Pinchart
> <laurent.pinchart+renesas@ideasonboard.com>
>    ---
>     drivers/of/dynamic.c | 222 ++++++++++++++++++++++++++++++++++
>     include/linux/of.h   | 328 +++++++++++++++++++++++++++++++++++++++++++++
>     2 files changed, 550 insertions(+)
> 
> My original scan of version 2 and then the email in this series had me
> questioning whether the two open issues from patch 2 had been addressed
> (yes, patch 0/16 did explicitly mention that 2 review comments had been
> taken into account, so you can point at my poor reading comprehension).

All points dully noted, I'll make sure not to include similar patches in the 
middle of a driver series should I need to rework a patch series previously 
posted by someone else.

I assume your comments also apply to patches to drivers/of/ that are developed 
as part of a drivers series, not merely pulled in it ? I have mixed feelings 
about this, as in many subsystems changes to the core require at least one 
user. Always splitting core and driver changes in separate series would thus 
often be impractical.

I doubt we will find a single process that will suit all subsystem 
maintainers, so maybe this should be a per-subsystem decision ?

> (3) This is totally unrelated to your patch series, but I'm leaving a bread
> crumb trail here.  Rob pointed you at a patch series that was not the most
> recent version.  When this patch series appears again, there already is
> a version 3 subset of it, but it is still labeled as being version 2, and
> also has outstanding un-addressed issues.  ("[PATCH v2 1/2] of: dynamic:
> Add __of_node_dupv()", 11/04/16)

Ah, thank you for the information. I was looking for a v3 but missed that one 
as it was labeled v2.

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 00/16] R-Car DU: Convert LVDS code to bridge driver
  2018-02-23  9:25       ` Laurent Pinchart
@ 2018-02-23 19:35         ` Frank Rowand
  0 siblings, 0 replies; 33+ messages in thread
From: Frank Rowand @ 2018-02-23 19:35 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Marek Vasut, Laurent Pinchart, Wolfram Sang, Koen Kooi,
	Pantelis Antoniou, linux-kernel, dri-devel, linux-renesas-soc,
	devicetree, linux-i2c, Matt Porter, Guenter Roeck

On 02/23/18 01:25, Laurent Pinchart wrote:
> Hi Frank,
> 
> On Friday, 23 February 2018 05:20:43 EET Frank Rowand wrote:
>> On 02/22/18 02:25, Laurent Pinchart wrote:
>>> On Thursday, 22 February 2018 08:07:14 EET Frank Rowand wrote:
>>>> 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.
>>>
>>> Because Rob asked for the driver-local implementation of the property add
>>> function to be replaced by Pantelis' series. I want to get the LVDS
>>> changes in v4.17 and asked Rob whether I could then take the OF changeset
>>> patches merged through the DRM tree, and he didn't object. If that causes
>>> an issue I'll switch back to the driver-local implementation to get the
>>> driver changes merged, split the OF changeset series out, and then move
>>> to the OF changeset API once merged. Would you prefer that ?
>>
>> You have already created a new version of the R-Car patches without the
>> set of patches that I was objecting to here.  So this is somewhat just
>> an academic comment.
>>
>> As I mentioned in the v6 thread, I am coming back here to clean up loose
>> ends, and explain why I had the reaction I had.  Basically, this is a
>> process issue to me.
>>
>> (1) The patch set from Pantelis is "hidden" in the driver patch series.
>> When viewing collapsed threads (which is my normal mode to avoid getting
>> overwhelmed by the vast volume of email I scan), the Pantelis patch set
>> is totally invisible.  If the R-Car driver patch series had not had me
>> on the to: list, there is a very good chance I would not have noticed
>> it.  Or noticed in a more delayed time frame.  And the same applies to
>> anyone else who might be subscribed to the devicetree mail list.  If
>> the Pantelis patch series was split out into a separate patch set then
>> it would be more visible on the list.
>>
>> (2) There is no good way to indicate in the email subject lines for
>> the Pantelis patches that they are version 3 of the series, since
>> they are also version 4 of the R-Car patch series.  If one reads the
>> patch 0/0 header carefully, and/or the other Pantelis patch comment
>> headers carefully, and then does a little detective work, it is
>> possible to find version 2 of the Pantelis patch series, and thus
>> be able to track the full history.  If just glancing at each
>> individual patch email subject, scanning the patch comment, and
>> spending more time reading the body of the patch, it would be
>> very easy to overlook the existence of the previous versions on
>> the mail list.
>>
>> (2b) Small quibble:  if the Pantelis patches were in a separate series,
>> with v3 in the subject header, then you would have normally put
>> a "changes since v2" section in the patch comment header, which would
>> have been more visible and less cryptic than what you wrote, which was:
>>
>>    ...
>>
>>    Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
>>    [Fixed memory leak in __of_changeset_add_update_property_copy()]
>>    Signed-off-by: Laurent Pinchart
>> <laurent.pinchart+renesas@ideasonboard.com>
>>    ---
>>     drivers/of/dynamic.c | 222 ++++++++++++++++++++++++++++++++++
>>     include/linux/of.h   | 328 +++++++++++++++++++++++++++++++++++++++++++++
>>     2 files changed, 550 insertions(+)
>>
>> My original scan of version 2 and then the email in this series had me
>> questioning whether the two open issues from patch 2 had been addressed
>> (yes, patch 0/16 did explicitly mention that 2 review comments had been
>> taken into account, so you can point at my poor reading comprehension).
> 
> All points dully noted, I'll make sure not to include similar patches in the 
> middle of a driver series should I need to rework a patch series previously 
> posted by someone else.
> 
> I assume your comments also apply to patches to drivers/of/ that are developed 
> as part of a drivers series, not merely pulled in it ?

Yes, as a normal process.  Of course we find cases where following normal process
causes problems, and it can be better to do something different in those cases.


> I have mixed feelings 
> about this, as in many subsystems changes to the core require at least one 
> user. Always splitting core and driver changes in separate series would thus 
> often be impractical.

If at all reasonable, I would still prefer to see two patch series for this
case.  I would expect both series to be upstreamed by a single maintainer
(with the two involved maintainers coordinating this process).  I have not
thought this through in any detail, so I don't know if I'm missing a bunch
of issues.  One issue that I can think of is clearly documenting which
version of the subsystem patch series any given version of the driver patch
series depends upon.  That should be easy to note in patch 0 of the driver
series.  Do you have experience with any other issues in this scenario?

This sounds like a good topic to include in the "Guide to Being A Linux Kernel
Maintainer".


> I doubt we will find a single process that will suit all subsystem 
> maintainers, so maybe this should be a per-subsystem decision ?

Totally agree.  And even for me, I see it as a guideline or normal process,
with exceptions made where it makes sense.


>> (3) This is totally unrelated to your patch series, but I'm leaving a bread
>> crumb trail here.  Rob pointed you at a patch series that was not the most
>> recent version.  When this patch series appears again, there already is
>> a version 3 subset of it, but it is still labeled as being version 2, and
>> also has outstanding un-addressed issues.  ("[PATCH v2 1/2] of: dynamic:
>> Add __of_node_dupv()", 11/04/16)
> 
> Ah, thank you for the information. I was looking for a v3 but missed that one 
> as it was labeled v2.
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2018-02-23 19:35 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-20 23:10 [PATCH v4 00/16] R-Car DU: Convert LVDS code to bridge driver Laurent Pinchart
2018-02-20 23:10 ` [PATCH v4 01/16] dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings Laurent Pinchart
2018-02-21  8:35   ` Sergei Shtylyov
2018-02-21 19:50     ` Laurent Pinchart
2018-02-20 23:10 ` [PATCH v4 02/16] dt-bindings: display: renesas: Deprecate LVDS support in the DU bindings Laurent Pinchart
2018-02-20 23:10 ` [PATCH v4 03/16] of: dynamic: Add __of_node_dupv() Laurent Pinchart
2018-02-21 10:26   ` Geert Uytterhoeven
2018-02-21 12:15     ` Laurent Pinchart
2018-02-21 23:16   ` Rob Herring
2018-02-20 23:10 ` [PATCH v4 04/16] of: changesets: Introduce changeset helper methods Laurent Pinchart
2018-02-21 10:21   ` Geert Uytterhoeven
2018-02-21 12:19     ` Laurent Pinchart
2018-02-21 15:23     ` Rob Herring
2018-02-21 15:46       ` Geert Uytterhoeven
2018-02-21 23:14   ` Rob Herring
2018-02-20 23:10 ` [PATCH v4 05/16] of: changeset: Add of_changeset_node_move method Laurent Pinchart
2018-02-21 23:20   ` Rob Herring
2018-02-21 23:40     ` Laurent Pinchart
2018-02-20 23:10 ` [PATCH v4 06/16] of: unittest: changeset helpers Laurent Pinchart
2018-02-21 23:10   ` Rob Herring
2018-02-21 23:39     ` Laurent Pinchart
2018-02-21 23:49       ` Rob Herring
2018-02-20 23:10 ` [PATCH v4 07/16] i2c: demux: Use changeset helpers for clarity Laurent Pinchart
2018-02-21  8:06   ` Wolfram Sang
2018-02-20 23:10 ` [PATCH v4 08/16] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes Laurent Pinchart
2018-02-21 23:28   ` Rob Herring
2018-02-21 23:54     ` Laurent Pinchart
2018-02-21 16:39 ` [PATCH v4 00/16] R-Car DU: Convert LVDS code to bridge driver Simon Horman
2018-02-22  6:07 ` Frank Rowand
2018-02-22 10:25   ` Laurent Pinchart
2018-02-23  3:20     ` Frank Rowand
2018-02-23  9:25       ` Laurent Pinchart
2018-02-23 19:35         ` Frank Rowand

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).