linux-renesas-soc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC 00/11] GMSL: Initial RDACM21 support
@ 2019-12-16 17:16 Jacopo Mondi
  2019-12-16 17:16 ` [RFC 01/11] fixup! DNI: Debug Jacopo Mondi
                   ` (10 more replies)
  0 siblings, 11 replies; 18+ messages in thread
From: Jacopo Mondi @ 2019-12-16 17:16 UTC (permalink / raw)
  To: kieran.bingham+renesas, laurent.pinchart, niklas.soderlund
  Cc: Jacopo Mondi, linux-renesas-soc

Hello Renesas multimedia,
   this series provides an initial support for RDACM21 camera modules,
which like the RDACM20 includes a MAX9271 serializer together with an OV490 ISP
chip and an OV10640 imager.

The series is based on the latest development from Kieran (gmsl/dev tag on
Kieran's kernel.org remote).

The series includes
- 4 fixes to be applied on top of Kieran's branch. Almost all of them have been
  reported by the dt schema validator.
- 05/11 which converts the proposed RDACM20 bindings to yaml
- 06/11 which breaks out MAX9271 handling from RDACM20 driver
- 07->09 which modified the max9286 driver to support remote communications
  with RDACM21 as well as RDACM20
- 10/11 which is an initial attempt to verify communication with RDACM21 by
  reading the ISP chip ID.
  At this stage, I augmented the RDACM20 driver to support both RDACM20 and 21.
  This defeats the purpose of 06/11 but I'm not sure which direction is better
  here, hence the RFC and the request for feedback.
  One driver per camera module, or a single driver with multiple compatibles?
  Please note that, as per RDACM20, the initial support for video capture
  operations will be limited to 1 resolution and one format, as the ISP+imager
  are configured by reading an on-chip EEPROM.
- Finally, enable RDACM21 in the Eagle DTS to test the example.

I'm very much interested in feedbacks on how to advance development for the
RDACM21 module.

Thanks
   j

Jacopo Mondi (11):
  fixup! DNI: Debug
  fixup! arm64: dts: renesas: salvator-x: Add MAX9286 expansion board
  fixup! arm64: dts: renesas: eagle: Provide Eagle FAKRA dynamic overlay
  fixup! arm64: dts: renesas: eagle: Provide MAX9286 GMSL deserialiser
  fixup! dt-bindings: media: i2c: Add bindings for IMI RDACM20
  media: i2c: Break out max9271 from rdacm20 driver
  media: i2c: max9286: Move notifiers operations
  media: i2c: max9286: Move link setup to completion
  media: i2c: max9286: Expand reverse chanenl amplitude
  WIP: media: i2c: rdacm20: Add RDACM21 support
  arm64: boot: dts: Eagle: Enable RDACM21

 .../bindings/media/i2c/imi,rdacm20.txt        |  66 ---
 .../bindings/media/i2c/imi,rdacm20.yaml       | 116 +++++
 arch/arm64/boot/dts/renesas/eagle-fakra.dtsi  |  12 +-
 .../arm64/boot/dts/renesas/r8a77970-eagle.dts |   3 -
 .../boot/dts/renesas/salvator-x-max9286.dtsi  |  16 +-
 drivers/media/i2c/Makefile                    |   3 +-
 drivers/media/i2c/max9271.c                   | 212 +++++++++
 drivers/media/i2c/max9271.h                   |  84 ++++
 drivers/media/i2c/max9286.c                   | 168 ++++---
 drivers/media/i2c/rdacm20.c                   | 433 +++++++-----------
 10 files changed, 693 insertions(+), 420 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/media/i2c/imi,rdacm20.txt
 create mode 100644 Documentation/devicetree/bindings/media/i2c/imi,rdacm20.yaml
 create mode 100644 drivers/media/i2c/max9271.c
 create mode 100644 drivers/media/i2c/max9271.h

--
2.24.0


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

* [RFC 01/11] fixup! DNI: Debug
  2019-12-16 17:16 [RFC 00/11] GMSL: Initial RDACM21 support Jacopo Mondi
@ 2019-12-16 17:16 ` Jacopo Mondi
  2019-12-16 22:34   ` Kieran Bingham
  2019-12-16 17:16 ` [RFC 02/11] fixup! arm64: dts: renesas: salvator-x: Add MAX9286 expansion board Jacopo Mondi
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 18+ messages in thread
From: Jacopo Mondi @ 2019-12-16 17:16 UTC (permalink / raw)
  To: kieran.bingham+renesas, laurent.pinchart, niklas.soderlund
  Cc: Jacopo Mondi, linux-renesas-soc

---
 drivers/media/i2c/max9286.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
index cb327c030081..ab84f0fa66dc 100644
--- a/drivers/media/i2c/max9286.c
+++ b/drivers/media/i2c/max9286.c
@@ -912,7 +912,7 @@ static int max9286_gpio_get(struct gpio_chip *chip, unsigned int offset)
 	struct max9286_priv *priv = gpiochip_get_data(chip);
 
 	dev_err(&priv->client->dev,
-		"GPIOSET: Offset %d, Value %d gpio_state = 0x%lx",
+		"GPIOSET: Offset %d, Value %ld gpio_state = 0x%lx",
 		offset, priv->gpio_state & BIT(offset),
 		MAX9286_0X0F_RESERVED | priv->gpio_state);
 
-- 
2.24.0


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

* [RFC 02/11] fixup! arm64: dts: renesas: salvator-x: Add MAX9286 expansion board
  2019-12-16 17:16 [RFC 00/11] GMSL: Initial RDACM21 support Jacopo Mondi
  2019-12-16 17:16 ` [RFC 01/11] fixup! DNI: Debug Jacopo Mondi
@ 2019-12-16 17:16 ` Jacopo Mondi
  2019-12-16 22:37   ` Kieran Bingham
  2019-12-16 17:16 ` [RFC 03/11] fixup! arm64: dts: renesas: eagle: Provide Eagle FAKRA dynamic overlay Jacopo Mondi
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 18+ messages in thread
From: Jacopo Mondi @ 2019-12-16 17:16 UTC (permalink / raw)
  To: kieran.bingham+renesas, laurent.pinchart, niklas.soderlund
  Cc: Jacopo Mondi, linux-renesas-soc

Fixes DTC warning:
r8a7795-es1-salvator-x.dt.yaml: camera@31: reg: [[49, 65, 81]] is too short
---
 .../boot/dts/renesas/salvator-x-max9286.dtsi     | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/salvator-x-max9286.dtsi b/arch/arm64/boot/dts/renesas/salvator-x-max9286.dtsi
index cfa3ecb6b2ae..0cc4c82b512f 100644
--- a/arch/arm64/boot/dts/renesas/salvator-x-max9286.dtsi
+++ b/arch/arm64/boot/dts/renesas/salvator-x-max9286.dtsi
@@ -191,7 +191,7 @@ i2c@0 {
 #ifdef MAXIM_CAMERA0
 				camera@31 {
 					compatible = MAXIM_CAMERA0;
-					reg = <0x31 0x41 0x51>;
+					reg = <0x31>, <0x41>, <0x51>;
 
 					port {
 						rdacm20_out0: endpoint {
@@ -211,7 +211,7 @@ i2c@1 {
 #ifdef MAXIM_CAMERA1
 				camera@32 {
 					compatible = MAXIM_CAMERA1;
-					reg = <0x32 0x42 0x52>;
+					reg = <0x32>, <0x42>, <0x52>;
 					port {
 						rdacm20_out1: endpoint {
 							remote-endpoint = <&max9286_in1>;
@@ -229,7 +229,7 @@ i2c@2 {
 #ifdef MAXIM_CAMERA2
 				camera@33 {
 					compatible = MAXIM_CAMERA2;
-					reg = <0x33 0x43 0x53>;
+					reg = <0x33>, <0x43>, <0x53>;
 					port {
 						rdacm20_out2: endpoint {
 							remote-endpoint = <&max9286_in2>;
@@ -247,7 +247,7 @@ i2c@3 {
 #ifdef MAXIM_CAMERA3
 				camera@34 {
 					compatible = MAXIM_CAMERA3;
-					reg = <0x34 0x44 0x54>;
+					reg = <0x34>, <0x44>, <0x54>;
 					port {
 						rdacm20_out3: endpoint {
 							remote-endpoint = <&max9286_in3>;
@@ -326,7 +326,7 @@ i2c@0 {
 #ifdef MAXIM_CAMERA4
 				camera@35 {
 					compatible = MAXIM_CAMERA4;
-					reg = <0x35 0x45 0x55>;
+					reg = <0x35>, <0x45>, <0x55>;
 					port {
 						rdacm20_out4: endpoint {
 							remote-endpoint = <&max9286_in4>;
@@ -344,7 +344,7 @@ i2c@1 {
 #ifdef MAXIM_CAMERA5
 				camera@36 {
 					compatible = MAXIM_CAMERA5;
-					reg = <0x36 0x46 0x56>;
+					reg = <0x36>, <0x46>, <0x56>;
 					port {
 						rdacm20_out5: endpoint {
 							remote-endpoint = <&max9286_in5>;
@@ -362,7 +362,7 @@ i2c@2 {
 #ifdef MAXIM_CAMERA6
 				camera@37 {
 					compatible = MAXIM_CAMERA6;
-					reg = <0x37 0x47 0x57>;
+					reg = <0x37>, <0x47>, <0x57>;
 					port {
 						rdacm20_out6: endpoint {
 							remote-endpoint = <&max9286_in6>;
@@ -380,7 +380,7 @@ i2c@3 {
 #ifdef MAXIM_CAMERA7
 				camera@38 {
 					compatible = MAXIM_CAMERA7;
-					reg = <0x38 0x48 0x58>;
+					reg = <0x38>, <0x48>, <0x58>;
 					port {
 						rdacm20_out7: endpoint {
 							remote-endpoint = <&max9286_in7>;
-- 
2.24.0


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

* [RFC 03/11] fixup! arm64: dts: renesas: eagle: Provide Eagle FAKRA dynamic overlay
  2019-12-16 17:16 [RFC 00/11] GMSL: Initial RDACM21 support Jacopo Mondi
  2019-12-16 17:16 ` [RFC 01/11] fixup! DNI: Debug Jacopo Mondi
  2019-12-16 17:16 ` [RFC 02/11] fixup! arm64: dts: renesas: salvator-x: Add MAX9286 expansion board Jacopo Mondi
@ 2019-12-16 17:16 ` Jacopo Mondi
  2019-12-16 22:40   ` Kieran Bingham
  2019-12-16 17:16 ` [RFC 04/11] fixup! arm64: dts: renesas: eagle: Provide MAX9286 GMSL deserialiser Jacopo Mondi
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 18+ messages in thread
From: Jacopo Mondi @ 2019-12-16 17:16 UTC (permalink / raw)
  To: kieran.bingham+renesas, laurent.pinchart, niklas.soderlund
  Cc: Jacopo Mondi, linux-renesas-soc

Fixes DTC warning
r8a77970-eagle.dt.yaml: camera@51: reg: [[81, 97]] is too short
---
 arch/arm64/boot/dts/renesas/eagle-fakra.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/eagle-fakra.dtsi b/arch/arm64/boot/dts/renesas/eagle-fakra.dtsi
index 1f3aeb49e4d9..d30d0f25e60f 100644
--- a/arch/arm64/boot/dts/renesas/eagle-fakra.dtsi
+++ b/arch/arm64/boot/dts/renesas/eagle-fakra.dtsi
@@ -63,7 +63,7 @@ i2c@0 {
 
 			camera@51 {
 				compatible = EAGLE_CAMERA0;
-				reg = <0x51 0x61>;
+				reg = <0x51>, <0x61>;
 
 				port {
 					fakra_con0: endpoint {
@@ -80,7 +80,7 @@ i2c@1 {
 
 			camera@52 {
 				compatible = EAGLE_CAMERA1;
-				reg = <0x52 0x62>;
+				reg = <0x52>, <0x62>;
 
 				port {
 					fakra_con1: endpoint {
@@ -97,7 +97,7 @@ i2c@2 {
 
 			camera@53 {
 				compatible = EAGLE_CAMERA2;
-				reg = <0x53 0x63>;
+				reg = <0x53>, <0x63>;
 
 				port {
 					fakra_con2: endpoint {
@@ -114,7 +114,7 @@ i2c@3 {
 
 			camera@54 {
 				compatible = EAGLE_CAMERA3;
-				reg = <0x54 0x64>;
+				reg = <0x54>, <0x64>;
 
 				port {
 					fakra_con3: endpoint {
-- 
2.24.0


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

* [RFC 04/11] fixup! arm64: dts: renesas: eagle: Provide MAX9286 GMSL deserialiser
  2019-12-16 17:16 [RFC 00/11] GMSL: Initial RDACM21 support Jacopo Mondi
                   ` (2 preceding siblings ...)
  2019-12-16 17:16 ` [RFC 03/11] fixup! arm64: dts: renesas: eagle: Provide Eagle FAKRA dynamic overlay Jacopo Mondi
@ 2019-12-16 17:16 ` Jacopo Mondi
  2019-12-16 22:41   ` Kieran Bingham
  2019-12-16 17:16 ` [RFC 05/11] fixup! dt-bindings: media: i2c: Add bindings for IMI RDACM20 Jacopo Mondi
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 18+ messages in thread
From: Jacopo Mondi @ 2019-12-16 17:16 UTC (permalink / raw)
  To: kieran.bingham+renesas, laurent.pinchart, niklas.soderlund
  Cc: Jacopo Mondi, linux-renesas-soc

Fixes DTC warning:
r8a77970-eagle.dts:236.29-335.4: Warning (avoid_unnecessary_addr_size): /soc/i2c@e66d0000/gmsl-deserializer@48: unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
---
 arch/arm64/boot/dts/renesas/r8a77970-eagle.dts | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
index c8cd548b7981..614f8d68d991 100644
--- a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
@@ -263,9 +263,6 @@ gmsl: gmsl-deserializer@48 {
 		compatible = "maxim,max9286";
 		reg = <0x48>;
 
-		#address-cells = <1>;
-		#size-cells = <0>;
-
 		/* eagle-pca9654-max9286-pwdn */
 		enable-gpios = <&io_expander 0 GPIO_ACTIVE_HIGH>;
 
-- 
2.24.0


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

* [RFC 05/11] fixup! dt-bindings: media: i2c: Add bindings for IMI RDACM20
  2019-12-16 17:16 [RFC 00/11] GMSL: Initial RDACM21 support Jacopo Mondi
                   ` (3 preceding siblings ...)
  2019-12-16 17:16 ` [RFC 04/11] fixup! arm64: dts: renesas: eagle: Provide MAX9286 GMSL deserialiser Jacopo Mondi
@ 2019-12-16 17:16 ` Jacopo Mondi
  2019-12-16 22:42   ` Laurent Pinchart
  2019-12-16 17:16 ` [RFC 06/11] media: i2c: Break out max9271 from rdacm20 driver Jacopo Mondi
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 18+ messages in thread
From: Jacopo Mondi @ 2019-12-16 17:16 UTC (permalink / raw)
  To: kieran.bingham+renesas, laurent.pinchart, niklas.soderlund
  Cc: Jacopo Mondi, linux-renesas-soc

---
 .../bindings/media/i2c/imi,rdacm20.txt        |  66 ----------
 .../bindings/media/i2c/imi,rdacm20.yaml       | 113 ++++++++++++++++++
 2 files changed, 113 insertions(+), 66 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/media/i2c/imi,rdacm20.txt
 create mode 100644 Documentation/devicetree/bindings/media/i2c/imi,rdacm20.yaml

diff --git a/Documentation/devicetree/bindings/media/i2c/imi,rdacm20.txt b/Documentation/devicetree/bindings/media/i2c/imi,rdacm20.txt
deleted file mode 100644
index 4731aafed63f..000000000000
--- a/Documentation/devicetree/bindings/media/i2c/imi,rdacm20.txt
+++ /dev/null
@@ -1,66 +0,0 @@
-IMI D&D RDACM20 Automotive Camera Platform
-------------------------------------------
-
-The IMI D&D RDACM20 is a GMSL-compatible camera designed for automotive
-applications. It encloses a Maxim Integrated MAX9271 GMSL serializer, an
-Omnivision OV10635 camera sensor and an embedded MCU, and connects to a remote
-GMSL endpoint through a coaxial cable.
-
-                                                     IMI RDACM20
- ---------------                               --------------------------------
-|      GMSL     |   <---  Video Stream        |       <- Video--------\        |
-|               |< ====== GMSL Link ======== >|MAX9271<- I2C bus-> <-->OV10635 |
-| de-serializer |   <---  I2C messages --->   |                   \<-->MCU     |
- ---------------                               --------------------------------
-
-The RDACM20 transmits video data generated by the embedded camera sensor on the
-GMSL serial channel to a remote GMSL de-serializer, as well as it receives and
-transmits I2C messages encapsulated in the GMSL bidirectional control channel.
-
-All I2C traffic received on the GMSL link not directed to the serializer is
-propagated on the local I2C bus to the embedded camera sensor and MCU. All
-I2C traffic generated on the local I2C bus not directed to the serializer is
-propagated to the remote de-serializer encapsulated in the GMSL control channel.
-
-The RDACM20 DT node should be a direct child of the GMSL Deserializer's I2C bus
-corresponding to the GMSL link that the camera is attached to.
-
-Required Properties:
-
-- compatible: Shall be "imi,rdacm20".
-- reg: I2C device addresses, the first to be assigned to the serializer
-  the second to be assigned to the camera sensor. An optional third address can
-  be provided to specify the MCU address if present.
-
-Connection to the remote GMSL endpoint are modelled using the OF graph bindings
-in accordance with the video interface bindings defined in
-Documentation/devicetree/bindings/media/video-interfaces.txt.
-
-The device node contains a single "port" child node with a single "endpoint"
-sub-device.
-
-Required endpoint properties:
-
-- remote-endpoint: phandle to the remote GMSL endpoint sub-node in the remote
-  node port.
-
-Example:
--------
-
-	i2c@0 {
-		#address-cells = <1>;
-		#size-cells = <0>;
-		reg = <0>;
-
-		camera@51 {
-			compatible = "imi,rdacm20";
-			reg = <0x31 0x41 0x51>;
-
-			port {
-				rdacm20_out0: endpoint {
-					remote-endpoint = <&max9286_in0>;
-				};
-			};
-
-		};
-	};
diff --git a/Documentation/devicetree/bindings/media/i2c/imi,rdacm20.yaml b/Documentation/devicetree/bindings/media/i2c/imi,rdacm20.yaml
new file mode 100644
index 000000000000..76740e285f44
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/imi,rdacm20.yaml
@@ -0,0 +1,113 @@
+# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
+# Copyright (C) 2019 Renesas Electronics Corp.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/imi,rdacm20.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title:  IMI D&D RDACM20 Automotive Camera Platform
+
+maintainers:
+  - Jacopo Mondi <jacopo+renesas@jmondi.org>
+  - Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
+  - Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
+  - Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
+
+description: -|
+  The IMI D&D RDACM20 is a GMSL-compatible camera designed for automotive
+  applications. It encloses a Maxim Integrated MAX9271 GMSL serializer, an
+  Omnivision OV10635 camera sensor and an embedded MCU, and connects to a remote
+  GMSL endpoint through a coaxial cable.
+
+                                                   IMI RDACM20
+  +---------------+                        +--------------------------------+
+  |      GMSL     |   <- Video Stream      |       <- Video--------\        |
+  |               |< === GMSL Link ====== >|MAX9271<- I2C bus-> <-->OV10635 |
+  | de-serializer |   <- I2C messages ->   |                   \<-->MCU     |
+  +---------------+                        +--------------------------------+
+
+  The RDACM20 transmits video data generated by the embedded camera sensor on
+  the GMSL serial channel to a remote GMSL de-serializer, as well as it receives
+  and transmits I2C messages encapsulated in the GMSL bidirectional control
+  channel.
+
+  All I2C traffic received on the GMSL link not directed to the serializer is
+  propagated on the local I2C bus to the embedded camera sensor and MCU. All I2C
+  traffic generated on the local I2C bus not directed to the serializer is
+  propagated to the remote de-serializer encapsulated in the GMSL control
+  channel.
+
+  The RDACM20 DT node should be a direct child of the GMSL Deserializer's I2C
+  bus corresponding to the GMSL link that the camera is attached to.
+
+properties:
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+  compatible:
+    const: imi,rdacm20
+
+  reg:
+    description: -|
+      I2C device addresses, the first to be assigned to the serializer the
+      second to be assigned to the camera sensor. An optional third address can
+      be provided to specify the MCU address if present.
+    minItems: 2
+    maxItems: 3
+
+  port:
+    type: object
+    additionalProperties: false
+    description: -|
+      Connection to the remote GMSL endpoint are modelled using the OF graph
+      bindings in accordance with the video interface bindings defined in
+      Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+      The device node contains a single "port" child node with a single
+      "endpoint" sub-device.
+
+    properties:
+      endpoint:
+        type: object
+        additionalProperties: false
+
+        properties:
+          remote-endpoint:
+            description: -|
+              phandle to the remote GMSL endpoint sub-node in the remote node
+              port.
+            maxItems: 1
+
+        required:
+          - remote-endpoint
+
+    required:
+      - endpoint
+
+required:
+  - compatible
+  - reg
+  - port
+
+examples:
+  - |
+    i2c@e66d8000 {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      reg = <0 0xe66d8000 0 0x40>;
+
+      camera@31 {
+        compatible = "imi,rdacm20";
+        reg = <0x31>, <0x41>, <0x51>;
+
+        port {
+          rdacm20_out0: endpoint {
+            remote-endpoint = <&max9286_in0>;
+          };
+        };
+      };
+    };
-- 
2.24.0


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

* [RFC 06/11] media: i2c: Break out max9271 from rdacm20 driver
  2019-12-16 17:16 [RFC 00/11] GMSL: Initial RDACM21 support Jacopo Mondi
                   ` (4 preceding siblings ...)
  2019-12-16 17:16 ` [RFC 05/11] fixup! dt-bindings: media: i2c: Add bindings for IMI RDACM20 Jacopo Mondi
@ 2019-12-16 17:16 ` Jacopo Mondi
  2019-12-16 17:16 ` [RFC 07/11] media: i2c: max9286: Move notifiers operations Jacopo Mondi
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Jacopo Mondi @ 2019-12-16 17:16 UTC (permalink / raw)
  To: kieran.bingham+renesas, laurent.pinchart, niklas.soderlund
  Cc: Jacopo Mondi, linux-renesas-soc

Break out from the RDACMD20 camera module driver functions to handle
Maxim MAX9271 GMSL serializer. Several GMSL-compatible camera modules
use MAX9271 as embedded serializer and breaking it out to a library
module allows reusage across different camera module drivers.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/i2c/Makefile  |   3 +-
 drivers/media/i2c/max9271.c | 212 ++++++++++++++++++++++++++
 drivers/media/i2c/max9271.h |  84 +++++++++++
 drivers/media/i2c/rdacm20.c | 286 +++++-------------------------------
 4 files changed, 332 insertions(+), 253 deletions(-)
 create mode 100644 drivers/media/i2c/max9271.c
 create mode 100644 drivers/media/i2c/max9271.h

diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index c4f47ebe498f..693b7d742827 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -118,6 +118,7 @@ obj-$(CONFIG_VIDEO_IMX319)	+= imx319.o
 obj-$(CONFIG_VIDEO_IMX355)	+= imx355.o
 obj-$(CONFIG_VIDEO_ST_MIPID02) += st-mipid02.o
 obj-$(CONFIG_VIDEO_MAX9286)	+= max9286.o
-obj-$(CONFIG_VIDEO_RDACM20)	+= rdacm20.o
+rdacm20-camera_module-objs	:= rdacm20.o max9271.o
+obj-$(CONFIG_VIDEO_RDACM20)	+= rdacm20-camera_module.o
 
 obj-$(CONFIG_SDR_MAX2175) += max2175.o
diff --git a/drivers/media/i2c/max9271.c b/drivers/media/i2c/max9271.c
new file mode 100644
index 000000000000..c6a9def09fcd
--- /dev/null
+++ b/drivers/media/i2c/max9271.c
@@ -0,0 +1,212 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * IMI RDACM20 GMSL Camera Driver
+ *
+ * Copyright (C) 2017-2018 Jacopo Mondi
+ * Copyright (C) 2017-2018 Kieran Bingham
+ * Copyright (C) 2017-2018 Laurent Pinchart
+ * Copyright (C) 2017-2018 Niklas Söderlund
+ * Copyright (C) 2016 Renesas Electronics Corporation
+ * Copyright (C) 2015 Cogent Embedded, Inc.
+ *
+ * This file exports functions to control Maxim MAX9271 GMSL serializer
+ * chip. This is not a self-contained driver, as MAX9271 is usually embedded in
+ * camera modules with at least one image sensor and optional additional
+ * components, such as uController units or ISPs/DSPs.
+ *
+ * Driver for the camera modules (ie rdacm20) are expected to use functions
+ * exported from this library file to maximize code re-use across drivers.
+ */
+#include <linux/delay.h>
+#include <linux/i2c.h>
+
+#include "max9271.h"
+
+static int max9271_read(struct max9271_device *dev, u8 reg)
+{
+	int ret;
+
+	dev_dbg(&dev->client->dev, "%s(0x%02x)\n", __func__, reg);
+
+	ret = i2c_smbus_read_byte_data(dev->client, reg);
+	if (ret < 0)
+		dev_dbg(&dev->client->dev,
+			"%s: register 0x%02x read failed (%d)\n",
+			__func__, reg, ret);
+
+	return ret;
+}
+
+static int max9271_write(struct max9271_device *dev, u8 reg, u8 val)
+{
+	int ret;
+
+	dev_dbg(&dev->client->dev, "%s(0x%02x, 0x%02x)\n", __func__, reg, val);
+
+	ret = i2c_smbus_write_byte_data(dev->client, reg, val);
+	if (ret < 0)
+		dev_err(&dev->client->dev,
+			"%s: register 0x%02x write failed (%d)\n",
+			__func__, reg, ret);
+
+	return ret;
+}
+
+/*
+ * max9271_pclk_detect() - Detect valid pixel clock from image sensor
+ *
+ * Wait up to 10ms for a valid pixel clock.
+ *
+ * Returns 0 for success, < 0 for pixel clock not properly detected
+ */
+static int max9271_pclk_detect(struct max9271_device *dev)
+{
+	unsigned int i;
+	int ret;
+
+	for (i = 0; i < 100; i++) {
+		ret = max9271_read(dev, 0x15);
+		if (ret < 0)
+			return ret;
+
+		if (ret & MAX9271_PCLKDET)
+			return 0;
+
+		usleep_range(50, 100);
+	}
+
+	dev_err(&dev->client->dev, "Unable to detect valid pixel clock\n");
+	return -EIO;
+}
+
+int max9271_s_stream(struct max9271_device *dev, bool enable)
+{
+	int ret;
+
+	if (enable) {
+		ret = max9271_pclk_detect(dev);
+		if (ret)
+			return ret;
+
+		/* Enable the serial link. */
+		max9271_write(dev, 0x04, MAX9271_SEREN | MAX9271_REVCCEN |
+			      MAX9271_FWDCCEN);
+	} else {
+		/* Disable the serial link. */
+		max9271_write(dev, 0x04, MAX9271_CLINKEN | MAX9271_REVCCEN |
+			      MAX9271_FWDCCEN);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(max9271_s_stream);
+
+int max9271_configure_i2c(struct max9271_device *dev)
+{
+	/*
+	 * Configure the I2C bus:
+	 *
+	 * - Enable high thresholds on the reverse channel
+	 * - Disable artificial ACK and set I2C speed
+	 */
+	max9271_write(dev, 0x08, MAX9271_REV_HIVTH);
+	usleep_range(5000, 8000);
+
+	max9271_write(dev, 0x0d, MAX9271_I2CSLVSH_469NS_234NS |
+		      MAX9271_I2CSLVTO_1024US | MAXIM_I2C_SPEED);
+	usleep_range(5000, 8000);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(max9271_configure_i2c);
+
+int max9271_configure_gmsl_link(struct max9271_device *dev)
+{
+	/*
+	 * Disable the serial link and enable the configuration link to allow
+	 * the control channel to operate in a low-speed mode in the absence of
+	 * the serial link clock.
+	 */
+	max9271_write(dev, 0x04, MAX9271_CLINKEN | MAX9271_REVCCEN |
+		      MAX9271_FWDCCEN);
+
+	/*
+	 * The serializer temporarily disables the reverse control channel for
+	 * 350µs after starting/stopping the forward serial link, but the
+	 * deserializer synchronization time isn't clearly documented.
+	 *
+	 * According to the serializer datasheet we should wait 3ms, while
+	 * according to the deserializer datasheet we should wait 5ms.
+	 *
+	 * Short delays here appear to show bit-errors in the writes following.
+	 * Therefore a conservative delay seems best here.
+	 */
+	usleep_range(5000, 8000);
+
+	/*
+	 * Configure the GMSL link:
+	 *
+	 * - Double input mode, high data rate, 24-bit mode
+	 * - Latch input data on PCLKIN rising edge
+	 * - Enable HS/VS encoding
+	 * - 1-bit parity error detection
+	 */
+	max9271_write(dev, 0x07, MAX9271_DBL | MAX9271_HVEN |
+		      MAX9271_EDC_1BIT_PARITY);
+	usleep_range(5000, 8000);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(max9271_configure_gmsl_link);
+
+int max9271_set_gpio(struct max9271_device *dev, u8 val)
+{
+	int ret = max9271_write(dev, 0x0f, val);
+	if (ret < 0) {
+		dev_err(&dev->client->dev, "Failed to set gpio (%d)\n", ret);
+		return ret;
+	}
+
+	usleep_range(10000, 20000);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(max9271_set_gpio);
+
+int max9271_verify_id(struct max9271_device *dev)
+{
+	int ret;
+
+	ret = max9271_read(dev, 0x1e);
+	if (ret < 0) {
+		dev_err(&dev->client->dev, "MAX9271 ID read failed (%d)\n",
+			ret);
+		return ret;
+	}
+
+	if (ret != MAX9271_ID) {
+		dev_err(&dev->client->dev, "MAX9271 ID mismatch (0x%02x)\n",
+			ret);
+		return -ENXIO;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(max9271_verify_id);
+
+int max9271_set_address(struct max9271_device *dev, u8 addr)
+{
+	int ret;
+
+	ret = max9271_write(dev, 0x00, addr << 1);
+	if (ret < 0) {
+		dev_err(&dev->client->dev,
+			"MAX9271 I2C address change failed (%d)\n", ret);
+		return ret;
+	}
+	dev->client->addr = addr;
+	usleep_range(3500, 5000);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(max9271_set_address);
diff --git a/drivers/media/i2c/max9271.h b/drivers/media/i2c/max9271.h
new file mode 100644
index 000000000000..bfe2d3b5f826
--- /dev/null
+++ b/drivers/media/i2c/max9271.h
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * IMI RDACM20 GMSL Camera Driver
+ *
+ * Copyright (C) 2017-2018 Jacopo Mondi
+ * Copyright (C) 2017-2018 Kieran Bingham
+ * Copyright (C) 2017-2018 Laurent Pinchart
+ * Copyright (C) 2017-2018 Niklas Söderlund
+ * Copyright (C) 2016 Renesas Electronics Corporation
+ * Copyright (C) 2015 Cogent Embedded, Inc.
+ */
+
+#include <linux/i2c.h>
+
+#define MAX9271_DEFAULT_ADDR	0x40
+
+#define MAXIM_I2C_I2C_SPEED_400KHZ	MAX9271_I2CMSTBT_339KBPS
+#define MAXIM_I2C_I2C_SPEED_100KHZ	MAX9271_I2CMSTBT_105KBPS
+#define MAXIM_I2C_SPEED			MAXIM_I2C_I2C_SPEED_100KHZ
+
+/* Register 0x04 */
+#define MAX9271_SEREN			BIT(7)
+#define MAX9271_CLINKEN			BIT(6)
+#define MAX9271_PRBSEN			BIT(5)
+#define MAX9271_SLEEP			BIT(4)
+#define MAX9271_INTTYPE_I2C		(0 << 2)
+#define MAX9271_INTTYPE_UART		(1 << 2)
+#define MAX9271_INTTYPE_NONE		(2 << 2)
+#define MAX9271_REVCCEN			BIT(1)
+#define MAX9271_FWDCCEN			BIT(0)
+/* Register 0x07 */
+#define MAX9271_DBL			BIT(7)
+#define MAX9271_DRS			BIT(6)
+#define MAX9271_BWS			BIT(5)
+#define MAX9271_ES			BIT(4)
+#define MAX9271_HVEN			BIT(2)
+#define MAX9271_EDC_1BIT_PARITY		(0 << 0)
+#define MAX9271_EDC_6BIT_CRC		(1 << 0)
+#define MAX9271_EDC_6BIT_HAMMING	(2 << 0)
+/* Register 0x08 */
+#define MAX9271_INVVS			BIT(7)
+#define MAX9271_INVHS			BIT(6)
+#define MAX9271_REV_LOGAIN		BIT(3)
+#define MAX9271_REV_HIVTH		BIT(0)
+/* Register 0x09 */
+#define MAX9271_ID			0x09
+/* Register 0x0d */
+#define MAX9271_I2CLOCACK		BIT(7)
+#define MAX9271_I2CSLVSH_1046NS_469NS	(3 << 5)
+#define MAX9271_I2CSLVSH_938NS_352NS	(2 << 5)
+#define MAX9271_I2CSLVSH_469NS_234NS	(1 << 5)
+#define MAX9271_I2CSLVSH_352NS_117NS	(0 << 5)
+#define MAX9271_I2CMSTBT_837KBPS	(7 << 2)
+#define MAX9271_I2CMSTBT_533KBPS	(6 << 2)
+#define MAX9271_I2CMSTBT_339KBPS	(5 << 2)
+#define MAX9271_I2CMSTBT_173KBPS	(4 << 2)
+#define MAX9271_I2CMSTBT_105KBPS	(3 << 2)
+#define MAX9271_I2CMSTBT_84KBPS		(2 << 2)
+#define MAX9271_I2CMSTBT_28KBPS		(1 << 2)
+#define MAX9271_I2CMSTBT_8KBPS		(0 << 2)
+#define MAX9271_I2CSLVTO_NONE		(3 << 0)
+#define MAX9271_I2CSLVTO_1024US		(2 << 0)
+#define MAX9271_I2CSLVTO_256US		(1 << 0)
+#define MAX9271_I2CSLVTO_64US		(0 << 0)
+/* Register 0x0f */
+#define MAX9271_GPIO5OUT		BIT(5)
+#define MAX9271_GPIO4OUT		BIT(4)
+#define MAX9271_GPIO3OUT		BIT(3)
+#define MAX9271_GPIO2OUT		BIT(2)
+#define MAX9271_GPIO1OUT		BIT(1)
+#define MAX9271_SETGPO			BIT(0)
+/* Register 0x15 */
+#define MAX9271_PCLKDET			BIT(0)
+
+struct max9271_device {
+	struct i2c_client *client;
+};
+
+int max9271_s_stream(struct max9271_device *dev, bool enable);
+int max9271_configure_i2c(struct max9271_device *dev);
+int max9271_configure_gmsl_link(struct max9271_device *dev);
+int max9271_set_gpio(struct max9271_device *dev, u8 val);
+int max9271_verify_id(struct max9271_device *dev);
+int max9271_set_address(struct max9271_device *dev, u8 addr);
diff --git a/drivers/media/i2c/rdacm20.c b/drivers/media/i2c/rdacm20.c
index 96bc2b793522..1a9c8d1f9484 100644
--- a/drivers/media/i2c/rdacm20.c
+++ b/drivers/media/i2c/rdacm20.c
@@ -28,69 +28,10 @@
 #include <media/v4l2-subdev.h>
 
 #include "rdacm20-ov10635.h"
+#include "max9271.h"
 
 #define RDACM20_SENSOR_HARD_RESET
 
-#define MAX9271_I2C_ADDRESS		0x40
-
-/* Register 0x04 */
-#define MAX9271_SEREN			BIT(7)
-#define MAX9271_CLINKEN			BIT(6)
-#define MAX9271_PRBSEN			BIT(5)
-#define MAX9271_SLEEP			BIT(4)
-#define MAX9271_INTTYPE_I2C		(0 << 2)
-#define MAX9271_INTTYPE_UART		(1 << 2)
-#define MAX9271_INTTYPE_NONE		(2 << 2)
-#define MAX9271_REVCCEN			BIT(1)
-#define MAX9271_FWDCCEN			BIT(0)
-/* Register 0x07 */
-#define MAX9271_DBL			BIT(7)
-#define MAX9271_DRS			BIT(6)
-#define MAX9271_BWS			BIT(5)
-#define MAX9271_ES			BIT(4)
-#define MAX9271_HVEN			BIT(2)
-#define MAX9271_EDC_1BIT_PARITY		(0 << 0)
-#define MAX9271_EDC_6BIT_CRC		(1 << 0)
-#define MAX9271_EDC_6BIT_HAMMING	(2 << 0)
-/* Register 0x08 */
-#define MAX9271_INVVS			BIT(7)
-#define MAX9271_INVHS			BIT(6)
-#define MAX9271_REV_LOGAIN		BIT(3)
-#define MAX9271_REV_HIVTH		BIT(0)
-/* Register 0x09 */
-#define MAX9271_ID			0x09
-/* Register 0x0d */
-#define MAX9271_I2CLOCACK		BIT(7)
-#define MAX9271_I2CSLVSH_1046NS_469NS	(3 << 5)
-#define MAX9271_I2CSLVSH_938NS_352NS	(2 << 5)
-#define MAX9271_I2CSLVSH_469NS_234NS	(1 << 5)
-#define MAX9271_I2CSLVSH_352NS_117NS	(0 << 5)
-#define MAX9271_I2CMSTBT_837KBPS	(7 << 2)
-#define MAX9271_I2CMSTBT_533KBPS	(6 << 2)
-#define MAX9271_I2CMSTBT_339KBPS	(5 << 2)
-#define MAX9271_I2CMSTBT_173KBPS	(4 << 2)
-#define MAX9271_I2CMSTBT_105KBPS	(3 << 2)
-#define MAX9271_I2CMSTBT_84KBPS		(2 << 2)
-#define MAX9271_I2CMSTBT_28KBPS		(1 << 2)
-#define MAX9271_I2CMSTBT_8KBPS		(0 << 2)
-#define MAX9271_I2CSLVTO_NONE		(3 << 0)
-#define MAX9271_I2CSLVTO_1024US		(2 << 0)
-#define MAX9271_I2CSLVTO_256US		(1 << 0)
-#define MAX9271_I2CSLVTO_64US		(0 << 0)
-/* Register 0x0f */
-#define MAX9271_GPIO5OUT		BIT(5)
-#define MAX9271_GPIO4OUT		BIT(4)
-#define MAX9271_GPIO3OUT		BIT(3)
-#define MAX9271_GPIO2OUT		BIT(2)
-#define MAX9271_GPIO1OUT		BIT(1)
-#define MAX9271_SETGPO			BIT(0)
-/* Register 0x15 */
-#define MAX9271_PCLKDET			BIT(0)
-
-#define MAXIM_I2C_I2C_SPEED_400KHZ	MAX9271_I2CMSTBT_339KBPS
-#define MAXIM_I2C_I2C_SPEED_100KHZ	MAX9271_I2CMSTBT_105KBPS
-#define MAXIM_I2C_SPEED			MAXIM_I2C_I2C_SPEED_100KHZ
-
 #define OV10635_I2C_ADDRESS		0x30
 
 #define OV10635_SOFTWARE_RESET		0x0103
@@ -106,7 +47,8 @@
 /* #define OV10635_FORMAT			MEDIA_BUS_FMT_UYVY10_2X10 */
 
 struct rdacm20_device {
-	struct i2c_client		*client;
+	struct device			*dev;
+	struct max9271_device		*serializer;
 	struct i2c_client		*sensor;
 	struct v4l2_subdev		sd;
 	struct media_pad		pad;
@@ -123,36 +65,6 @@ static inline struct rdacm20_device *i2c_to_rdacm20(struct i2c_client *client)
 	return sd_to_rdacm20(i2c_get_clientdata(client));
 }
 
-static int max9271_read(struct rdacm20_device *dev, u8 reg)
-{
-	int ret;
-
-	dev_dbg(&dev->client->dev, "%s(0x%02x)\n", __func__, reg);
-
-	ret = i2c_smbus_read_byte_data(dev->client, reg);
-	if (ret < 0)
-		dev_dbg(&dev->client->dev,
-			"%s: register 0x%02x read failed (%d)\n",
-			__func__, reg, ret);
-
-	return ret;
-}
-
-static int max9271_write(struct rdacm20_device *dev, u8 reg, u8 val)
-{
-	int ret;
-
-	dev_dbg(&dev->client->dev, "%s(0x%02x, 0x%02x)\n", __func__, reg, val);
-
-	ret = i2c_smbus_write_byte_data(dev->client, reg, val);
-	if (ret < 0)
-		dev_err(&dev->client->dev,
-			"%s: register 0x%02x write failed (%d)\n",
-			__func__, reg, ret);
-
-	return ret;
-}
-
 static int ov10635_read16(struct rdacm20_device *dev, u16 reg)
 {
 	u8 buf[2] = { reg >> 8, reg & 0xff };
@@ -163,8 +75,7 @@ static int ov10635_read16(struct rdacm20_device *dev, u16 reg)
 		ret = i2c_master_recv(dev->sensor, buf, 2);
 
 	if (ret < 0) {
-		dev_dbg(&dev->client->dev,
-			"%s: register 0x%04x read failed (%d)\n",
+		dev_dbg(dev->dev, "%s: register 0x%04x read failed (%d)\n",
 			__func__, reg, ret);
 		return ret;
 	}
@@ -177,7 +88,7 @@ static int __ov10635_write(struct rdacm20_device *dev, u16 reg, u8 val)
 	u8 buf[3] = { reg >> 8, reg & 0xff, val };
 	int ret;
 
-	dev_dbg(&dev->client->dev, "%s(0x%04x, 0x%02x)\n", __func__, reg, val);
+	dev_dbg(dev->dev, "%s(0x%04x, 0x%02x)\n", __func__, reg, val);
 
 	ret = i2c_master_send(dev->sensor, buf, 3);
 	return ret < 0 ? ret : 0;
@@ -189,8 +100,7 @@ static int ov10635_write(struct rdacm20_device *dev, u16 reg, u8 val)
 
 	ret = __ov10635_write(dev, reg, val);
 	if (ret < 0)
-		dev_err(&dev->client->dev,
-			"%s: register 0x%04x write failed (%d)\n",
+		dev_err(dev->dev, "%s: register 0x%04x write failed (%d)\n",
 			__func__, reg, ret);
 
 	return ret;
@@ -206,7 +116,7 @@ static int ov10635_set_regs(struct rdacm20_device *dev,
 	for (i = 0; i < nr_regs; i++) {
 		ret = __ov10635_write(dev, regs[i].reg, regs[i].val);
 		if (ret) {
-			dev_err(&dev->client->dev,
+			dev_err(dev->dev,
 				"%s: register %u (0x%04x) write failed (%d)\n",
 				__func__, i, regs[i].reg, ret);
 			return ret;
@@ -216,53 +126,11 @@ static int ov10635_set_regs(struct rdacm20_device *dev,
 	return 0;
 }
 
-/*
- * rdacm20_pclk_detect() - Detect valid pixel clock from image sensor
- *
- * Wait up to 10ms for a valid pixel clock.
- *
- * Returns 0 for success, < 0 for pixel clock not properly detected
- */
-static int rdacm20_pclk_detect(struct rdacm20_device *dev)
-{
-	unsigned int i;
-	int ret;
-
-	for (i = 0; i < 100; i++) {
-		ret = max9271_read(dev, 0x15);
-		if (ret < 0)
-			return ret;
-
-		if (ret & MAX9271_PCLKDET)
-			return 0;
-
-		usleep_range(50, 100);
-	}
-
-	dev_err(&dev->client->dev, "Unable to detect valid pixel clock\n");
-	return -EIO;
-}
-
 static int rdacm20_s_stream(struct v4l2_subdev *sd, int enable)
 {
 	struct rdacm20_device *dev = sd_to_rdacm20(sd);
-	int ret;
-
-	if (enable) {
-		ret = rdacm20_pclk_detect(dev);
-		if (ret)
-			return ret;
-
-		/* Enable the serial link. */
-		max9271_write(dev, 0x04, MAX9271_SEREN | MAX9271_REVCCEN |
-			      MAX9271_FWDCCEN);
-	} else {
-		/* Disable the serial link. */
-		max9271_write(dev, 0x04, MAX9271_CLINKEN | MAX9271_REVCCEN |
-			      MAX9271_FWDCCEN);
-	}
 
-	return 0;
+	return max9271_s_stream(dev->serializer, enable);
 }
 
 static int rdacm20_enum_mbus_code(struct v4l2_subdev *sd,
@@ -313,108 +181,15 @@ static struct v4l2_subdev_ops rdacm20_subdev_ops = {
 	.pad		= &rdacm20_subdev_pad_ops,
 };
 
-static int max9271_configure_i2c(struct rdacm20_device *dev)
-{
-	/*
-	 * Configure the I2C bus:
-	 *
-	 * - Enable high thresholds on the reverse channel
-	 * - Disable artificial ACK and set I2C speed
-	 */
-	max9271_write(dev, 0x08, MAX9271_REV_HIVTH);
-	usleep_range(5000, 8000);
-
-	max9271_write(dev, 0x0d, MAX9271_I2CSLVSH_469NS_234NS |
-		      MAX9271_I2CSLVTO_1024US | MAXIM_I2C_SPEED);
-	usleep_range(5000, 8000);
-
-	return 0;
-}
-
-static int max9271_configure_gmsl_link(struct rdacm20_device *dev)
-{
-	/*
-	 * Disable the serial link and enable the configuration link to allow
-	 * the control channel to operate in a low-speed mode in the absence of
-	 * the serial link clock.
-	 */
-	max9271_write(dev, 0x04, MAX9271_CLINKEN | MAX9271_REVCCEN |
-		      MAX9271_FWDCCEN);
-
-	/*
-	 * The serializer temporarily disables the reverse control channel for
-	 * 350µs after starting/stopping the forward serial link, but the
-	 * deserializer synchronization time isn't clearly documented.
-	 *
-	 * According to the serializer datasheet we should wait 3ms, while
-	 * according to the deserializer datasheet we should wait 5ms.
-	 *
-	 * Short delays here appear to show bit-errors in the writes following.
-	 * Therefore a conservative delay seems best here.
-	 */
-	usleep_range(5000, 8000);
-
-	/*
-	 * Configure the GMSL link:
-	 *
-	 * - Double input mode, high data rate, 24-bit mode
-	 * - Latch input data on PCLKIN rising edge
-	 * - Enable HS/VS encoding
-	 * - 1-bit parity error detection
-	 */
-	max9271_write(dev, 0x07, MAX9271_DBL | MAX9271_HVEN |
-		      MAX9271_EDC_1BIT_PARITY);
-	usleep_range(5000, 8000);
-
-	return 0;
-}
-
-static int max9271_verify_id(struct rdacm20_device *dev)
-{
-	int ret;
-
-	ret = max9271_read(dev, 0x1e);
-	if (ret < 0) {
-		dev_err(&dev->client->dev, "MAX9271 ID read failed (%d)\n",
-			ret);
-		return ret;
-	}
-
-	if (ret != MAX9271_ID) {
-		dev_err(&dev->client->dev, "MAX9271 ID mismatch (0x%02x)\n",
-			ret);
-		return -ENXIO;
-	}
-
-	return 0;
-}
-
-static int max9271_configure_address(struct rdacm20_device *dev, u8 addr)
-{
-	int ret;
-
-	/* Change the MAX9271 I2C address. */
-	ret = max9271_write(dev, 0x00, addr << 1);
-	if (ret < 0) {
-		dev_err(&dev->client->dev,
-			"MAX9271 I2C address change failed (%d)\n", ret);
-		return ret;
-	}
-	dev->client->addr = addr;
-	usleep_range(3500, 5000);
-
-	return 0;
-}
-
 static int rdacm20_initialize(struct rdacm20_device *dev)
 {
 	u32 addrs[2];
 	int ret;
 
-	ret = of_property_read_u32_array(dev->client->dev.of_node, "reg",
+	ret = of_property_read_u32_array(dev->dev->of_node, "reg",
 					 addrs, ARRAY_SIZE(addrs));
 	if (ret < 0) {
-		dev_err(&dev->client->dev, "Invalid DT reg property\n");
+		dev_err(dev->dev, "Invalid DT reg property\n");
 		return -EINVAL;
 	}
 
@@ -423,41 +198,42 @@ static int rdacm20_initialize(struct rdacm20_device *dev)
 	 * the address specified in DT. Set the client address back to the
 	 * default for initial communication.
 	 */
+
 	/* Create a dummy device, configure to that device, then change the
 	 * address. Then delete it when the address is changed?
 	 */
-	dev->client->addr = MAX9271_I2C_ADDRESS;
+	dev->serializer->client->addr = MAX9271_DEFAULT_ADDR;
 
-	/* Verify communication with the MAX9271. */
-	i2c_smbus_read_byte(dev->client);	/* ping to wake-up */
+	/* Verify communication with the MAX9271: ping to wakeup. */
+	i2c_smbus_read_byte(dev->serializer->client);
 
 	/*
 	 *  Ensure that we have a good link configuration before attempting to
 	 *  identify the device.
 	 */
-	max9271_configure_i2c(dev);
-	max9271_configure_gmsl_link(dev);
+	max9271_configure_i2c(dev->serializer);
+	max9271_configure_gmsl_link(dev->serializer);
 
-	ret = max9271_verify_id(dev);
+	ret = max9271_verify_id(dev->serializer);
 	if (ret < 0)
 		return ret;
 
-	ret = max9271_configure_address(dev, addrs[0]);
+	ret = max9271_set_address(dev->serializer, addrs[0]);
 	if (ret < 0)
 		return ret;
 
 	/* Reset and verify communication with the OV10635. */
 #ifdef RDACM20_SENSOR_HARD_RESET
 	/* Cycle the OV10635 reset signal connected to the MAX9271 GPIO1. */
-	max9271_write(dev, 0x0f, 0xff & ~(MAX9271_GPIO1OUT | MAX9271_SETGPO));
-	usleep_range(10000, 20000);
-	max9271_write(dev, 0x0f, 0xff & ~MAX9271_SETGPO);
-	usleep_range(10000, 20000);
+	max9271_set_gpio(dev->serializer,
+			 0xff & ~(MAX9271_GPIO1OUT | MAX9271_SETGPO));
+	max9271_set_gpio(dev->serializer,
+			0xff & ~MAX9271_SETGPO);
 #else
 	/* Perform a software reset. */
 	ret = ov10635_write(dev, OV10635_SOFTWARE_RESET, 1);
 	if (ret < 0) {
-		dev_err(&dev->client->dev, "OV10635 reset failed (%d)\n", ret);
+		dev_err(dev->dev, "OV10635 reset failed (%d)\n", ret);
 		return -ENXIO;
 	}
 
@@ -466,24 +242,24 @@ static int rdacm20_initialize(struct rdacm20_device *dev)
 
 	ret = ov10635_read16(dev, OV10635_PID);
 	if (ret < 0) {
-		dev_err(&dev->client->dev, "OV10635 ID read failed (%d)\n",
+		dev_err(dev->dev, "OV10635 ID read failed (%d)\n",
 			ret);
 		return -ENXIO;
 	}
 
 	if (ret != OV10635_VERSION) {
-		dev_err(&dev->client->dev, "OV10635 ID mismatch (0x%04x)\n",
+		dev_err(dev->dev, "OV10635 ID mismatch (0x%04x)\n",
 			ret);
 		return -ENXIO;
 	}
 
-	dev_info(&dev->client->dev, "Identified MAX9271 + OV10635 device\n");
+	dev_info(dev->dev, "Identified MAX9271 + OV10635 device\n");
 
 	/* Change the sensor I2C address. */
 	ret = ov10635_write(dev, OV10635_SC_CMMN_SCCB_ID,
 			    (addrs[1] << 1) | OV10635_SC_CMMN_SCCB_ID_SELECT);
 	if (ret < 0) {
-		dev_err(&dev->client->dev,
+		dev_err(dev->dev,
 			"OV10635 I2C address change failed (%d)\n", ret);
 		return ret;
 	}
@@ -504,8 +280,14 @@ static int rdacm20_probe(struct i2c_client *client)
 	dev = devm_kzalloc(&client->dev, sizeof(*dev), GFP_KERNEL);
 	if (!dev)
 		return -ENOMEM;
+	dev->dev = &client->dev;
+
+	dev->serializer = devm_kzalloc(&client->dev, sizeof(*dev->serializer),
+				       GFP_KERNEL);
+	if (!dev->serializer)
+		return -ENOMEM;
 
-	dev->client = client;
+	dev->serializer->client = client;
 
 	/* Create the dummy I2C client for the sensor. */
 	dev->sensor = i2c_new_dummy(client->adapter, OV10635_I2C_ADDRESS);
-- 
2.24.0


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

* [RFC 07/11] media: i2c: max9286: Move notifiers operations
  2019-12-16 17:16 [RFC 00/11] GMSL: Initial RDACM21 support Jacopo Mondi
                   ` (5 preceding siblings ...)
  2019-12-16 17:16 ` [RFC 06/11] media: i2c: Break out max9271 from rdacm20 driver Jacopo Mondi
@ 2019-12-16 17:16 ` Jacopo Mondi
  2019-12-16 17:16 ` [RFC 08/11] media: i2c: max9286: Move link setup to completion Jacopo Mondi
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Jacopo Mondi @ 2019-12-16 17:16 UTC (permalink / raw)
  To: kieran.bingham+renesas, laurent.pinchart, niklas.soderlund
  Cc: Jacopo Mondi, linux-renesas-soc

In preparation to move the call to max9286_check_config_link() in
the notifier's bound callback, move the notifier's ops below the
max9286_check_config_link() function.

Cosmetic change, no functional changes intended.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/i2c/max9286.c | 114 ++++++++++++++++++------------------
 1 file changed, 57 insertions(+), 57 deletions(-)

diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
index ab84f0fa66dc..3c2c1f506983 100644
--- a/drivers/media/i2c/max9286.c
+++ b/drivers/media/i2c/max9286.c
@@ -445,63 +445,6 @@ static void max9286_configure_i2c(struct max9286_priv *priv, bool localack)
 	usleep_range(3000, 5000);
 }
 
-/* -----------------------------------------------------------------------------
- * V4L2 Subdev
- */
-
-static int max9286_notify_bound(struct v4l2_async_notifier *notifier,
-				struct v4l2_subdev *subdev,
-				struct v4l2_async_subdev *asd)
-{
-	struct max9286_priv *priv = sd_to_max9286(notifier->sd);
-	struct max9286_source *source = asd_to_max9286_source(asd);
-	unsigned int index = to_index(priv, source);
-	unsigned int src_pad;
-	int ret;
-
-	ret = media_entity_get_fwnode_pad(&subdev->entity,
-					  source->fwnode,
-					  MEDIA_PAD_FL_SOURCE);
-	if (ret < 0) {
-		dev_err(&priv->client->dev,
-			"Failed to find pad for %s\n", subdev->name);
-		return ret;
-	}
-
-	source->sd = subdev;
-	src_pad = ret;
-
-	ret = media_create_pad_link(&source->sd->entity, src_pad,
-				    &priv->sd.entity, index,
-				    MEDIA_LNK_FL_ENABLED |
-				    MEDIA_LNK_FL_IMMUTABLE);
-	if (ret) {
-		dev_err(&priv->client->dev,
-			"Unable to link %s:%u -> %s:%u\n",
-			source->sd->name, src_pad, priv->sd.name, index);
-		return ret;
-	}
-
-	dev_dbg(&priv->client->dev, "Bound %s pad: %u on index %u\n",
-		subdev->name, src_pad, index);
-
-	return 0;
-}
-
-static void max9286_notify_unbind(struct v4l2_async_notifier *notifier,
-				  struct v4l2_subdev *subdev,
-				  struct v4l2_async_subdev *asd)
-{
-	struct max9286_source *source = asd_to_max9286_source(asd);
-
-	source->sd = NULL;
-}
-
-static const struct v4l2_async_notifier_operations max9286_notify_ops = {
-	.bound = max9286_notify_bound,
-	.unbind = max9286_notify_unbind,
-};
-
 /*
  * max9286_check_config_link() - Detect and wait for configuration links
  *
@@ -602,6 +545,63 @@ static int max9286_check_video_links(struct max9286_priv *priv)
 	return 0;
 }
 
+/* -----------------------------------------------------------------------------
+ * V4L2 Subdev
+ */
+
+static int max9286_notify_bound(struct v4l2_async_notifier *notifier,
+				struct v4l2_subdev *subdev,
+				struct v4l2_async_subdev *asd)
+{
+	struct max9286_priv *priv = sd_to_max9286(notifier->sd);
+	struct max9286_source *source = asd_to_max9286_source(asd);
+	unsigned int index = to_index(priv, source);
+	unsigned int src_pad;
+	int ret;
+
+	ret = media_entity_get_fwnode_pad(&subdev->entity,
+					  source->fwnode,
+					  MEDIA_PAD_FL_SOURCE);
+	if (ret < 0) {
+		dev_err(&priv->client->dev,
+			"Failed to find pad for %s\n", subdev->name);
+		return ret;
+	}
+
+	source->sd = subdev;
+	src_pad = ret;
+
+	ret = media_create_pad_link(&source->sd->entity, src_pad,
+				    &priv->sd.entity, index,
+				    MEDIA_LNK_FL_ENABLED |
+				    MEDIA_LNK_FL_IMMUTABLE);
+	if (ret) {
+		dev_err(&priv->client->dev,
+			"Unable to link %s:%u -> %s:%u\n",
+			source->sd->name, src_pad, priv->sd.name, index);
+		return ret;
+	}
+
+	dev_dbg(&priv->client->dev, "Bound %s pad: %u on index %u\n",
+		subdev->name, src_pad, index);
+
+	return 0;
+}
+
+static void max9286_notify_unbind(struct v4l2_async_notifier *notifier,
+				  struct v4l2_subdev *subdev,
+				  struct v4l2_async_subdev *asd)
+{
+	struct max9286_source *source = asd_to_max9286_source(asd);
+
+	source->sd = NULL;
+}
+
+static const struct v4l2_async_notifier_operations max9286_notify_ops = {
+	.bound = max9286_notify_bound,
+	.unbind = max9286_notify_unbind,
+};
+
 static int max9286_s_stream(struct v4l2_subdev *sd, int enable)
 {
 	struct max9286_priv *priv = sd_to_max9286(sd);
-- 
2.24.0


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

* [RFC 08/11] media: i2c: max9286: Move link setup to completion
  2019-12-16 17:16 [RFC 00/11] GMSL: Initial RDACM21 support Jacopo Mondi
                   ` (6 preceding siblings ...)
  2019-12-16 17:16 ` [RFC 07/11] media: i2c: max9286: Move notifiers operations Jacopo Mondi
@ 2019-12-16 17:16 ` Jacopo Mondi
  2019-12-16 17:16 ` [RFC 09/11] media: i2c: max9286: Expand reverse chanenl amplitude Jacopo Mondi
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Jacopo Mondi @ 2019-12-16 17:16 UTC (permalink / raw)
  To: kieran.bingham+renesas, laurent.pinchart, niklas.soderlund
  Cc: Jacopo Mondi, linux-renesas-soc

The max9286 async notifier is a sub-notifier which does not support a
'complete' callback.

In order to properly complete the reverse channel configuration, we need
to perform operations after all remote serializers have completed their
probe.

Keep track of how many serializers have probed, and once all the
expected ones have completed its initialization:
1) Compensate the reverse channel high threshold
2) Verify that all expected control links are enabled
3) Disable i2c auto-ack

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/i2c/max9286.c | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
index 3c2c1f506983..f75c97ef87a8 100644
--- a/drivers/media/i2c/max9286.c
+++ b/drivers/media/i2c/max9286.c
@@ -166,6 +166,7 @@ struct max9286_priv {
 	unsigned int nsources;
 	unsigned int source_mask;
 	unsigned int route_mask;
+	unsigned int bound_sources;
 	unsigned int csi2_data_lanes;
 	struct max9286_source sources[MAX9286_NUM_GMSL];
 	struct v4l2_async_notifier notifier;
@@ -570,6 +571,7 @@ static int max9286_notify_bound(struct v4l2_async_notifier *notifier,
 
 	source->sd = subdev;
 	src_pad = ret;
+	priv->bound_sources |= BIT(index);
 
 	ret = media_create_pad_link(&source->sd->entity, src_pad,
 				    &priv->sd.entity, index,
@@ -585,6 +587,25 @@ static int max9286_notify_bound(struct v4l2_async_notifier *notifier,
 	dev_dbg(&priv->client->dev, "Bound %s pad: %u on index %u\n",
 		subdev->name, src_pad, index);
 
+	if (priv->bound_sources != priv->source_mask)
+		return 0;
+
+	/*
+	 * All enabled sources have probed and enabled their reverse control
+	 * channels:
+	 *
+	 * - Increase the reverse channel amplitude to 170mV to accommodate the
+	 *   high threshold enabled by the serializer driver.
+	 * - Verify all configuration links are properly detected
+	 * - Disable auto-ack as communication on the control channel are now
+	 *   stable.
+	 */
+	max9286_write(priv, 0x3b, MAX9286_REV_TRF(1) | MAX9286_REV_AMP(70) |
+		      MAX9286_REV_AMP_X);
+	usleep_range(2000, 2500);
+	max9286_check_config_link(priv, priv->source_mask);
+	max9286_configure_i2c(priv, false);
+
 	return 0;
 }
 
@@ -876,11 +897,6 @@ static int max9286_setup(struct max9286_priv *priv)
 	 */
 	usleep_range(2000, 5000);
 
-	/*
-	 * Check to see if the expected configuration links are up.
-	 */
-	max9286_check_config_link(priv, priv->source_mask);
-
 	return 0;
 }
 
@@ -1042,12 +1058,6 @@ static int max9286_init(struct device *dev, void *data)
 		goto err_subdev_unregister;
 	}
 
-	/*
-	 * Re-configure I2C with local acknowledge disabled after cameras
-	 * have probed.
-	 */
-	max9286_configure_i2c(priv, false);
-
 	/* Leave the mux channels disabled until they are selected. */
 	max9286_i2c_mux_close(priv);
 
-- 
2.24.0


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

* [RFC 09/11] media: i2c: max9286: Expand reverse chanenl amplitude
  2019-12-16 17:16 [RFC 00/11] GMSL: Initial RDACM21 support Jacopo Mondi
                   ` (7 preceding siblings ...)
  2019-12-16 17:16 ` [RFC 08/11] media: i2c: max9286: Move link setup to completion Jacopo Mondi
@ 2019-12-16 17:16 ` Jacopo Mondi
  2019-12-16 17:16 ` [RFC 10/11] WIP: media: i2c: rdacm20: Add RDACM21 support Jacopo Mondi
  2019-12-16 17:16 ` [RFC 11/11] arm64: boot: dts: Eagle: Enable RDACM21 Jacopo Mondi
  10 siblings, 0 replies; 18+ messages in thread
From: Jacopo Mondi @ 2019-12-16 17:16 UTC (permalink / raw)
  To: kieran.bingham+renesas, laurent.pinchart, niklas.soderlund
  Cc: Jacopo Mondi, linux-renesas-soc

Expand commen on the reverse channel amplitude compensation as its
handling differs between RDACM20 and RDACM21. Keep the current
register configuration as it works for both devices.

While at it, add a delay after reverse channel re-configuration as
suggested by the programming guide.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/i2c/max9286.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
index f75c97ef87a8..e9d3da72a381 100644
--- a/drivers/media/i2c/max9286.c
+++ b/drivers/media/i2c/max9286.c
@@ -857,12 +857,28 @@ static int max9286_setup(struct max9286_priv *priv)
 	 *
 	 * - Enable custom reverse channel configuration (through register 0x3f)
 	 *   and set the first pulse length to 35 clock cycles.
-	 * - Increase the reverse channel amplitude to 170mV to accommodate the
-	 *   high threshold enabled by the serializer driver.
+	 * - FIXME: Set the reverse channel amplitude to 70mV ( + 100 for
+	 *   RDACM20)
+	 *
+	 *   The RDACM21 and RDACM20 camera modules this driver has been
+	 *   tested against would need to be handled differently here.
+	 *
+	 *   RDACM20 has an MCU which performs an initial programming, most
+	 *   probably enabling the reverse channel and high-threshold
+	 *   compensation during startup. It needs then to be interoperated
+	 *   with the deserializer reverse channel amplitude already compensated
+	 *   to 170mV (70mV + 100 mV).
+	 *
+	 *   RDACM21 does not need that and reverse channel could have been
+	 *   compensated after all serializers have probed.
+	 *
+	 *   Without this early compensation RDACM20 fails to probe, but RDACM21
+	 *   shows slightly less reliable communications.
 	 */
 	max9286_write(priv, 0x3f, MAX9286_EN_REV_CFG | MAX9286_REV_FLEN(35));
 	max9286_write(priv, 0x3b, MAX9286_REV_TRF(1) | MAX9286_REV_AMP(70) |
 		      MAX9286_REV_AMP_X);
+	usleep_range(2000, 2500);
 
 	/*
 	 * Enable GMSL links, mask unused ones and autodetect link
-- 
2.24.0


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

* [RFC 10/11] WIP: media: i2c: rdacm20: Add RDACM21 support
  2019-12-16 17:16 [RFC 00/11] GMSL: Initial RDACM21 support Jacopo Mondi
                   ` (8 preceding siblings ...)
  2019-12-16 17:16 ` [RFC 09/11] media: i2c: max9286: Expand reverse chanenl amplitude Jacopo Mondi
@ 2019-12-16 17:16 ` Jacopo Mondi
  2019-12-16 17:16 ` [RFC 11/11] arm64: boot: dts: Eagle: Enable RDACM21 Jacopo Mondi
  10 siblings, 0 replies; 18+ messages in thread
From: Jacopo Mondi @ 2019-12-16 17:16 UTC (permalink / raw)
  To: kieran.bingham+renesas, laurent.pinchart, niklas.soderlund
  Cc: Jacopo Mondi, linux-renesas-soc

Add support to the rdacm20 driver for the RDACM21 camera module.

The RDACM21 camera modules includes a max9271 serializer, an OV490 ISP
in conjuction with an OV10640 imager.

This patch implements partial support, up to the point where
communications with the ISP chip are verified.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 .../bindings/media/i2c/imi,rdacm20.yaml       |   5 +-
 drivers/media/i2c/rdacm20.c                   | 183 +++++++++++++++---
 2 files changed, 156 insertions(+), 32 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/imi,rdacm20.yaml b/Documentation/devicetree/bindings/media/i2c/imi,rdacm20.yaml
index 76740e285f44..18f8d1fbdbae 100644
--- a/Documentation/devicetree/bindings/media/i2c/imi,rdacm20.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/imi,rdacm20.yaml
@@ -48,7 +48,10 @@ properties:
     const: 0
 
   compatible:
-    const: imi,rdacm20
+    items:
+      - enum:
+        - imi,rdacm20
+        - imi,rdacm21
 
   reg:
     description: -|
diff --git a/drivers/media/i2c/rdacm20.c b/drivers/media/i2c/rdacm20.c
index 1a9c8d1f9484..0431bbb5cbd3 100644
--- a/drivers/media/i2c/rdacm20.c
+++ b/drivers/media/i2c/rdacm20.c
@@ -20,6 +20,7 @@
 #include <linux/init.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/slab.h>
 #include <linux/videodev2.h>
 
@@ -33,6 +34,10 @@
 #define RDACM20_SENSOR_HARD_RESET
 
 #define OV10635_I2C_ADDRESS		0x30
+#define ATTINY85_I2C_ADDRESS		0x50
+
+#define OV10640_I2C_ADDRESS		0x30
+#define OV490_I2C_ADDRESS		0x24
 
 #define OV10635_SOFTWARE_RESET		0x0103
 #define OV10635_PID			0x300a
@@ -46,15 +51,29 @@
 #define OV10635_FORMAT			MEDIA_BUS_FMT_UYVY8_2X8
 /* #define OV10635_FORMAT			MEDIA_BUS_FMT_UYVY10_2X10 */
 
+#define OV490_PID			0x300a
+#define OV490_VER			0x300b
+#define OV490_ID_VAL			0x0490
+#define OV490_ID(_p, _v)		((_p) << 8 | (_v) & 0xff)
+
+struct rdacm_data;
+
 struct rdacm20_device {
 	struct device			*dev;
 	struct max9271_device		*serializer;
 	struct i2c_client		*sensor;
+	const struct rdacm_data		*data;
 	struct v4l2_subdev		sd;
 	struct media_pad		pad;
 	struct v4l2_ctrl_handler	ctrls;
 };
 
+struct rdacm_data {
+	char *devname;
+	int (*sensor_probe)(struct rdacm20_device *dev);
+	unsigned int default_addrs[2];
+};
+
 static inline struct rdacm20_device *sd_to_rdacm20(struct v4l2_subdev *sd)
 {
 	return container_of(sd, struct rdacm20_device, sd);
@@ -181,8 +200,9 @@ static struct v4l2_subdev_ops rdacm20_subdev_ops = {
 	.pad		= &rdacm20_subdev_pad_ops,
 };
 
-static int rdacm20_initialize(struct rdacm20_device *dev)
+static int rdacm20_sensor_probe(struct rdacm20_device *dev)
 {
+	const struct rdacm_data *data = dev->data;
 	u32 addrs[2];
 	int ret;
 
@@ -193,34 +213,7 @@ static int rdacm20_initialize(struct rdacm20_device *dev)
 		return -EINVAL;
 	}
 
-	/*
-	 * FIXME: The MAX9271 boots at a default address that we will change to
-	 * the address specified in DT. Set the client address back to the
-	 * default for initial communication.
-	 */
-
-	/* Create a dummy device, configure to that device, then change the
-	 * address. Then delete it when the address is changed?
-	 */
-	dev->serializer->client->addr = MAX9271_DEFAULT_ADDR;
-
-	/* Verify communication with the MAX9271: ping to wakeup. */
-	i2c_smbus_read_byte(dev->serializer->client);
-
-	/*
-	 *  Ensure that we have a good link configuration before attempting to
-	 *  identify the device.
-	 */
-	max9271_configure_i2c(dev->serializer);
-	max9271_configure_gmsl_link(dev->serializer);
-
-	ret = max9271_verify_id(dev->serializer);
-	if (ret < 0)
-		return ret;
-
-	ret = max9271_set_address(dev->serializer, addrs[0]);
-	if (ret < 0)
-		return ret;
+	dev->sensor->addr = data->default_addrs[0];
 
 	/* Reset and verify communication with the OV10635. */
 #ifdef RDACM20_SENSOR_HARD_RESET
@@ -271,6 +264,106 @@ static int rdacm20_initialize(struct rdacm20_device *dev)
 				ARRAY_SIZE(ov10635_regs_wizard));
 }
 
+static int rdacm21_sensor_probe(struct rdacm20_device *dev)
+{
+	const struct rdacm_data *data = dev->data;
+	u32 addrs[2];
+	u8 pid, ver;
+	int ret;
+
+	ret = of_property_read_u32_array(dev->dev->of_node, "reg",
+					 addrs, ARRAY_SIZE(addrs));
+	if (ret < 0) {
+		dev_err(dev->dev, "Invalid DT reg property\n");
+		return -EINVAL;
+	}
+
+	/* Verify communications with OV490 by reading its product ID. */
+	dev->sensor->addr = data->default_addrs[1];
+	ret = ov10635_write(dev, 0xfffd, 0x80);
+	if (ret)
+		return ret;
+
+	ret = ov10635_write(dev, 0xfffe, 0x80);
+	if (ret)
+		return ret;
+
+	ret = ov10635_read16(dev, OV490_PID);
+	if (ret < 0) {
+		dev_err(dev->dev, "OV490 PID read failed (%d)\n", ret);
+		return ret;
+	}
+	pid = ret;
+
+	ret = ov10635_read16(dev, OV490_VER);
+	if (ret < 0) {
+		dev_err(dev->dev, "OV490 VERSION read failed (%d)\n", ret);
+		return ret;
+	}
+	ver = ret;
+
+	if (OV490_ID(pid, ver) != OV490_ID_VAL) {
+		dev_dbg(dev->dev, "OV490 ID mismatch: (0x%04x)\n",
+			OV490_ID(pid, ver));
+		return -ENODEV;
+	}
+
+	dev_info(dev->dev, "Identified MAX9271 + OV490 + OV10640 device\n");
+
+	/*
+	 * TODO: program MAX9271 to perform address translation to talk with
+	 * OV490 from the SoC side.
+	 */
+
+	return 0;
+}
+
+static int rdacm20_initialize(struct rdacm20_device *dev)
+{
+	u32 addrs[2];
+	int ret;
+
+	ret = of_property_read_u32_array(dev->dev->of_node, "reg",
+					 addrs, ARRAY_SIZE(addrs));
+	if (ret < 0) {
+		dev_err(dev->dev, "Invalid DT reg property\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * FIXME: The MAX9271 boots at a default address that we will change to
+	 * the address specified in DT. Set the client address back to the
+	 * default for initial communication.
+	 */
+
+	/* Create a dummy device, configure to that device, then change the
+	 * address. Then delete it when the address is changed?
+	 */
+	dev->serializer->client->addr = MAX9271_DEFAULT_ADDR;
+
+	/* Verify communication with the MAX9271: ping to wakeup. */
+	i2c_smbus_read_byte(dev->serializer->client);
+
+	/*
+	 *  Ensure that we have a good link configuration before attempting to
+	 *  identify the device.
+	 */
+	max9271_configure_gmsl_link(dev->serializer);
+	max9271_configure_i2c(dev->serializer);
+
+	ret = max9271_verify_id(dev->serializer);
+	if (ret < 0)
+		return ret;
+
+	ret = max9271_set_address(dev->serializer, addrs[0]);
+	if (ret < 0)
+		return ret;
+
+	return dev->data->sensor_probe(dev);
+
+	return 0;
+}
+
 static int rdacm20_probe(struct i2c_client *client)
 {
 	struct rdacm20_device *dev;
@@ -282,6 +375,9 @@ static int rdacm20_probe(struct i2c_client *client)
 		return -ENOMEM;
 	dev->dev = &client->dev;
 
+
+	dev->data = of_device_get_match_data(&client->dev);
+
 	dev->serializer = devm_kzalloc(&client->dev, sizeof(*dev->serializer),
 				       GFP_KERNEL);
 	if (!dev->serializer)
@@ -289,7 +385,10 @@ static int rdacm20_probe(struct i2c_client *client)
 
 	dev->serializer->client = client;
 
-	/* Create the dummy I2C client for the sensor. */
+	/*
+	 * Create the dummy I2C client for the sensor: the right i2c address
+	 * will be overwritten later at sensor initialization time.
+	 */
 	dev->sensor = i2c_new_dummy(client->adapter, OV10635_I2C_ADDRESS);
 	if (!dev->sensor) {
 		ret = -ENXIO;
@@ -303,6 +402,9 @@ static int rdacm20_probe(struct i2c_client *client)
 
 	/* Initialize and register the subdevice. */
 	v4l2_i2c_subdev_init(&dev->sd, client, &rdacm20_subdev_ops);
+	snprintf(dev->sd.name, sizeof(dev->sd.name), "%s %d-%04x",
+		 dev->data->devname, i2c_adapter_id(dev->sensor->adapter),
+		 dev->sensor->addr);
 	dev->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 
 	v4l2_ctrl_handler_init(&dev->ctrls, 1);
@@ -375,8 +477,27 @@ static void rdacm20_shutdown(struct i2c_client *client)
 	rdacm20_s_stream(&dev->sd, 0);
 }
 
+struct rdacm_data rdacm20_data = {
+	.devname = "rdacm20",
+	.sensor_probe = rdacm20_sensor_probe,
+	.default_addrs = {
+		OV10635_I2C_ADDRESS,
+		ATTINY85_I2C_ADDRESS,
+	},
+};
+
+struct rdacm_data rdacm21_data = {
+	.devname = "rdacm21",
+	.sensor_probe = rdacm21_sensor_probe,
+	.default_addrs = {
+		OV10635_I2C_ADDRESS,
+		OV490_I2C_ADDRESS,
+	},
+};
+
 static const struct of_device_id rdacm20_of_ids[] = {
-	{ .compatible = "imi,rdacm20", },
+	{ .compatible = "imi,rdacm20", .data = &rdacm20_data },
+	{ .compatible = "imi,rdacm21", .data = &rdacm21_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, rdacm20_of_ids);
-- 
2.24.0


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

* [RFC 11/11] arm64: boot: dts: Eagle: Enable RDACM21
  2019-12-16 17:16 [RFC 00/11] GMSL: Initial RDACM21 support Jacopo Mondi
                   ` (9 preceding siblings ...)
  2019-12-16 17:16 ` [RFC 10/11] WIP: media: i2c: rdacm20: Add RDACM21 support Jacopo Mondi
@ 2019-12-16 17:16 ` Jacopo Mondi
  10 siblings, 0 replies; 18+ messages in thread
From: Jacopo Mondi @ 2019-12-16 17:16 UTC (permalink / raw)
  To: kieran.bingham+renesas, laurent.pinchart, niklas.soderlund
  Cc: Jacopo Mondi, linux-renesas-soc

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 arch/arm64/boot/dts/renesas/eagle-fakra.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/eagle-fakra.dtsi b/arch/arm64/boot/dts/renesas/eagle-fakra.dtsi
index d30d0f25e60f..e2b39a4a13d5 100644
--- a/arch/arm64/boot/dts/renesas/eagle-fakra.dtsi
+++ b/arch/arm64/boot/dts/renesas/eagle-fakra.dtsi
@@ -19,8 +19,8 @@
 
 #define EAGLE_CAMERA0 "imi,rdacm20"
 #define EAGLE_CAMERA1 "imi,rdacm20"
-//#define EAGLE_CAMERA2 "imi,rdacm21"
-//#define EAGLE_CAMERA3 "imi,rdacm21"
+#define EAGLE_CAMERA2 "imi,rdacm21"
+#define EAGLE_CAMERA3 "imi,rdacm21"
 
 /* Define the endpoint links */
 #ifdef EAGLE_CAMERA0
-- 
2.24.0


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

* Re: [RFC 01/11] fixup! DNI: Debug
  2019-12-16 17:16 ` [RFC 01/11] fixup! DNI: Debug Jacopo Mondi
@ 2019-12-16 22:34   ` Kieran Bingham
  0 siblings, 0 replies; 18+ messages in thread
From: Kieran Bingham @ 2019-12-16 22:34 UTC (permalink / raw)
  To: Jacopo Mondi, laurent.pinchart, niklas.soderlund; +Cc: linux-renesas-soc

Hi Jacopo,

On 16/12/2019 17:16, Jacopo Mondi wrote:
> ---
>  drivers/media/i2c/max9286.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thanks, I've squashed this onto my branch.
--
Kieran

> 
> diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
> index cb327c030081..ab84f0fa66dc 100644
> --- a/drivers/media/i2c/max9286.c
> +++ b/drivers/media/i2c/max9286.c
> @@ -912,7 +912,7 @@ static int max9286_gpio_get(struct gpio_chip *chip, unsigned int offset)
>  	struct max9286_priv *priv = gpiochip_get_data(chip);
>  
>  	dev_err(&priv->client->dev,
> -		"GPIOSET: Offset %d, Value %d gpio_state = 0x%lx",
> +		"GPIOSET: Offset %d, Value %ld gpio_state = 0x%lx",
>  		offset, priv->gpio_state & BIT(offset),
>  		MAX9286_0X0F_RESERVED | priv->gpio_state);
>  
> 


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

* Re: [RFC 02/11] fixup! arm64: dts: renesas: salvator-x: Add MAX9286 expansion board
  2019-12-16 17:16 ` [RFC 02/11] fixup! arm64: dts: renesas: salvator-x: Add MAX9286 expansion board Jacopo Mondi
@ 2019-12-16 22:37   ` Kieran Bingham
  0 siblings, 0 replies; 18+ messages in thread
From: Kieran Bingham @ 2019-12-16 22:37 UTC (permalink / raw)
  To: Jacopo Mondi, laurent.pinchart, niklas.soderlund; +Cc: linux-renesas-soc

Hi Jacopo,

On 16/12/2019 17:16, Jacopo Mondi wrote:
> Fixes DTC warning:
> r8a7795-es1-salvator-x.dt.yaml: camera@31: reg: [[49, 65, 81]] is too short

Agreed, these should be separated.

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

Thanks - I'll squash this onto my gmsl/platform branch.

--
Kieran


> ---
>  .../boot/dts/renesas/salvator-x-max9286.dtsi     | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/renesas/salvator-x-max9286.dtsi b/arch/arm64/boot/dts/renesas/salvator-x-max9286.dtsi
> index cfa3ecb6b2ae..0cc4c82b512f 100644
> --- a/arch/arm64/boot/dts/renesas/salvator-x-max9286.dtsi
> +++ b/arch/arm64/boot/dts/renesas/salvator-x-max9286.dtsi
> @@ -191,7 +191,7 @@ i2c@0 {
>  #ifdef MAXIM_CAMERA0
>  				camera@31 {
>  					compatible = MAXIM_CAMERA0;
> -					reg = <0x31 0x41 0x51>;
> +					reg = <0x31>, <0x41>, <0x51>;
>  
>  					port {
>  						rdacm20_out0: endpoint {
> @@ -211,7 +211,7 @@ i2c@1 {
>  #ifdef MAXIM_CAMERA1
>  				camera@32 {
>  					compatible = MAXIM_CAMERA1;
> -					reg = <0x32 0x42 0x52>;
> +					reg = <0x32>, <0x42>, <0x52>;
>  					port {
>  						rdacm20_out1: endpoint {
>  							remote-endpoint = <&max9286_in1>;
> @@ -229,7 +229,7 @@ i2c@2 {
>  #ifdef MAXIM_CAMERA2
>  				camera@33 {
>  					compatible = MAXIM_CAMERA2;
> -					reg = <0x33 0x43 0x53>;
> +					reg = <0x33>, <0x43>, <0x53>;
>  					port {
>  						rdacm20_out2: endpoint {
>  							remote-endpoint = <&max9286_in2>;
> @@ -247,7 +247,7 @@ i2c@3 {
>  #ifdef MAXIM_CAMERA3
>  				camera@34 {
>  					compatible = MAXIM_CAMERA3;
> -					reg = <0x34 0x44 0x54>;
> +					reg = <0x34>, <0x44>, <0x54>;
>  					port {
>  						rdacm20_out3: endpoint {
>  							remote-endpoint = <&max9286_in3>;
> @@ -326,7 +326,7 @@ i2c@0 {
>  #ifdef MAXIM_CAMERA4
>  				camera@35 {
>  					compatible = MAXIM_CAMERA4;
> -					reg = <0x35 0x45 0x55>;
> +					reg = <0x35>, <0x45>, <0x55>;
>  					port {
>  						rdacm20_out4: endpoint {
>  							remote-endpoint = <&max9286_in4>;
> @@ -344,7 +344,7 @@ i2c@1 {
>  #ifdef MAXIM_CAMERA5
>  				camera@36 {
>  					compatible = MAXIM_CAMERA5;
> -					reg = <0x36 0x46 0x56>;
> +					reg = <0x36>, <0x46>, <0x56>;
>  					port {
>  						rdacm20_out5: endpoint {
>  							remote-endpoint = <&max9286_in5>;
> @@ -362,7 +362,7 @@ i2c@2 {
>  #ifdef MAXIM_CAMERA6
>  				camera@37 {
>  					compatible = MAXIM_CAMERA6;
> -					reg = <0x37 0x47 0x57>;
> +					reg = <0x37>, <0x47>, <0x57>;
>  					port {
>  						rdacm20_out6: endpoint {
>  							remote-endpoint = <&max9286_in6>;
> @@ -380,7 +380,7 @@ i2c@3 {
>  #ifdef MAXIM_CAMERA7
>  				camera@38 {
>  					compatible = MAXIM_CAMERA7;
> -					reg = <0x38 0x48 0x58>;
> +					reg = <0x38>, <0x48>, <0x58>;
>  					port {
>  						rdacm20_out7: endpoint {
>  							remote-endpoint = <&max9286_in7>;
> 


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

* Re: [RFC 03/11] fixup! arm64: dts: renesas: eagle: Provide Eagle FAKRA dynamic overlay
  2019-12-16 17:16 ` [RFC 03/11] fixup! arm64: dts: renesas: eagle: Provide Eagle FAKRA dynamic overlay Jacopo Mondi
@ 2019-12-16 22:40   ` Kieran Bingham
  0 siblings, 0 replies; 18+ messages in thread
From: Kieran Bingham @ 2019-12-16 22:40 UTC (permalink / raw)
  To: Jacopo Mondi, laurent.pinchart, niklas.soderlund; +Cc: linux-renesas-soc

Hi Jacopo,

On 16/12/2019 17:16, Jacopo Mondi wrote:
> Fixes DTC warning
> r8a77970-eagle.dt.yaml: camera@51: reg: [[81, 97]] is too short

Also agreed,

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

Thanks - I'll squash this onto my gmsl/platform branch.

> ---
>  arch/arm64/boot/dts/renesas/eagle-fakra.dtsi | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/renesas/eagle-fakra.dtsi b/arch/arm64/boot/dts/renesas/eagle-fakra.dtsi
> index 1f3aeb49e4d9..d30d0f25e60f 100644
> --- a/arch/arm64/boot/dts/renesas/eagle-fakra.dtsi
> +++ b/arch/arm64/boot/dts/renesas/eagle-fakra.dtsi
> @@ -63,7 +63,7 @@ i2c@0 {
>  
>  			camera@51 {
>  				compatible = EAGLE_CAMERA0;
> -				reg = <0x51 0x61>;
> +				reg = <0x51>, <0x61>;
>  
>  				port {
>  					fakra_con0: endpoint {
> @@ -80,7 +80,7 @@ i2c@1 {
>  
>  			camera@52 {
>  				compatible = EAGLE_CAMERA1;
> -				reg = <0x52 0x62>;
> +				reg = <0x52>, <0x62>;
>  
>  				port {
>  					fakra_con1: endpoint {
> @@ -97,7 +97,7 @@ i2c@2 {
>  
>  			camera@53 {
>  				compatible = EAGLE_CAMERA2;
> -				reg = <0x53 0x63>;
> +				reg = <0x53>, <0x63>;
>  
>  				port {
>  					fakra_con2: endpoint {
> @@ -114,7 +114,7 @@ i2c@3 {
>  
>  			camera@54 {
>  				compatible = EAGLE_CAMERA3;
> -				reg = <0x54 0x64>;
> +				reg = <0x54>, <0x64>;
>  
>  				port {
>  					fakra_con3: endpoint {
> 


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

* Re: [RFC 04/11] fixup! arm64: dts: renesas: eagle: Provide MAX9286 GMSL deserialiser
  2019-12-16 17:16 ` [RFC 04/11] fixup! arm64: dts: renesas: eagle: Provide MAX9286 GMSL deserialiser Jacopo Mondi
@ 2019-12-16 22:41   ` Kieran Bingham
  0 siblings, 0 replies; 18+ messages in thread
From: Kieran Bingham @ 2019-12-16 22:41 UTC (permalink / raw)
  To: Jacopo Mondi, laurent.pinchart, niklas.soderlund; +Cc: linux-renesas-soc

Hi Jacopo,

On 16/12/2019 17:16, Jacopo Mondi wrote:
> Fixes DTC warning:
> r8a77970-eagle.dts:236.29-335.4: Warning (avoid_unnecessary_addr_size): /soc/i2c@e66d0000/gmsl-deserializer@48: unnecessary #address-cells/#size-cells without "ranges" or child "reg" property

Aha, this must be a left over from moving the child nodes into an i2c-mux.


Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

Thanks - I'll squash this onto my gmsl/platform branch.

> ---
>  arch/arm64/boot/dts/renesas/r8a77970-eagle.dts | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
> index c8cd548b7981..614f8d68d991 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
> @@ -263,9 +263,6 @@ gmsl: gmsl-deserializer@48 {
>  		compatible = "maxim,max9286";
>  		reg = <0x48>;
>  
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -
>  		/* eagle-pca9654-max9286-pwdn */
>  		enable-gpios = <&io_expander 0 GPIO_ACTIVE_HIGH>;
>  
> 


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

* Re: [RFC 05/11] fixup! dt-bindings: media: i2c: Add bindings for IMI RDACM20
  2019-12-16 17:16 ` [RFC 05/11] fixup! dt-bindings: media: i2c: Add bindings for IMI RDACM20 Jacopo Mondi
@ 2019-12-16 22:42   ` Laurent Pinchart
  2020-02-14  9:07     ` Kieran Bingham
  0 siblings, 1 reply; 18+ messages in thread
From: Laurent Pinchart @ 2019-12-16 22:42 UTC (permalink / raw)
  To: Jacopo Mondi; +Cc: kieran.bingham+renesas, niklas.soderlund, linux-renesas-soc

Hi Jacopo,

Thank you for the patch.

On Mon, Dec 16, 2019 at 06:16:14PM +0100, Jacopo Mondi wrote:
> ---
>  .../bindings/media/i2c/imi,rdacm20.txt        |  66 ----------
>  .../bindings/media/i2c/imi,rdacm20.yaml       | 113 ++++++++++++++++++
>  2 files changed, 113 insertions(+), 66 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/media/i2c/imi,rdacm20.txt
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/imi,rdacm20.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/imi,rdacm20.txt b/Documentation/devicetree/bindings/media/i2c/imi,rdacm20.txt
> deleted file mode 100644
> index 4731aafed63f..000000000000
> --- a/Documentation/devicetree/bindings/media/i2c/imi,rdacm20.txt
> +++ /dev/null
> @@ -1,66 +0,0 @@
> -IMI D&D RDACM20 Automotive Camera Platform
> -------------------------------------------
> -
> -The IMI D&D RDACM20 is a GMSL-compatible camera designed for automotive
> -applications. It encloses a Maxim Integrated MAX9271 GMSL serializer, an
> -Omnivision OV10635 camera sensor and an embedded MCU, and connects to a remote
> -GMSL endpoint through a coaxial cable.
> -
> -                                                     IMI RDACM20
> - ---------------                               --------------------------------
> -|      GMSL     |   <---  Video Stream        |       <- Video--------\        |
> -|               |< ====== GMSL Link ======== >|MAX9271<- I2C bus-> <-->OV10635 |
> -| de-serializer |   <---  I2C messages --->   |                   \<-->MCU     |
> - ---------------                               --------------------------------
> -
> -The RDACM20 transmits video data generated by the embedded camera sensor on the
> -GMSL serial channel to a remote GMSL de-serializer, as well as it receives and
> -transmits I2C messages encapsulated in the GMSL bidirectional control channel.
> -
> -All I2C traffic received on the GMSL link not directed to the serializer is
> -propagated on the local I2C bus to the embedded camera sensor and MCU. All
> -I2C traffic generated on the local I2C bus not directed to the serializer is
> -propagated to the remote de-serializer encapsulated in the GMSL control channel.
> -
> -The RDACM20 DT node should be a direct child of the GMSL Deserializer's I2C bus
> -corresponding to the GMSL link that the camera is attached to.
> -
> -Required Properties:
> -
> -- compatible: Shall be "imi,rdacm20".
> -- reg: I2C device addresses, the first to be assigned to the serializer
> -  the second to be assigned to the camera sensor. An optional third address can
> -  be provided to specify the MCU address if present.
> -
> -Connection to the remote GMSL endpoint are modelled using the OF graph bindings
> -in accordance with the video interface bindings defined in
> -Documentation/devicetree/bindings/media/video-interfaces.txt.
> -
> -The device node contains a single "port" child node with a single "endpoint"
> -sub-device.
> -
> -Required endpoint properties:
> -
> -- remote-endpoint: phandle to the remote GMSL endpoint sub-node in the remote
> -  node port.
> -
> -Example:
> --------
> -
> -	i2c@0 {
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -		reg = <0>;
> -
> -		camera@51 {
> -			compatible = "imi,rdacm20";
> -			reg = <0x31 0x41 0x51>;
> -
> -			port {
> -				rdacm20_out0: endpoint {
> -					remote-endpoint = <&max9286_in0>;
> -				};
> -			};
> -
> -		};
> -	};
> diff --git a/Documentation/devicetree/bindings/media/i2c/imi,rdacm20.yaml b/Documentation/devicetree/bindings/media/i2c/imi,rdacm20.yaml
> new file mode 100644
> index 000000000000..76740e285f44
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/imi,rdacm20.yaml
> @@ -0,0 +1,113 @@
> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> +# Copyright (C) 2019 Renesas Electronics Corp.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/imi,rdacm20.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title:  IMI D&D RDACM20 Automotive Camera Platform
> +
> +maintainers:
> +  - Jacopo Mondi <jacopo+renesas@jmondi.org>
> +  - Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> +  - Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> +  - Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> +
> +description: -|
> +  The IMI D&D RDACM20 is a GMSL-compatible camera designed for automotive
> +  applications. It encloses a Maxim Integrated MAX9271 GMSL serializer, an
> +  Omnivision OV10635 camera sensor and an embedded MCU, and connects to a remote
> +  GMSL endpoint through a coaxial cable.
> +
> +                                                   IMI RDACM20
> +  +---------------+                        +--------------------------------+
> +  |      GMSL     |   <- Video Stream      |       <- Video--------\        |
> +  |               |< === GMSL Link ====== >|MAX9271<- I2C bus-> <-->OV10635 |
> +  | de-serializer |   <- I2C messages ->   |                   \<-->MCU     |
> +  +---------------+                        +--------------------------------+
> +
> +  The RDACM20 transmits video data generated by the embedded camera sensor on
> +  the GMSL serial channel to a remote GMSL de-serializer, as well as it receives
> +  and transmits I2C messages encapsulated in the GMSL bidirectional control
> +  channel.
> +
> +  All I2C traffic received on the GMSL link not directed to the serializer is
> +  propagated on the local I2C bus to the embedded camera sensor and MCU. All I2C
> +  traffic generated on the local I2C bus not directed to the serializer is
> +  propagated to the remote de-serializer encapsulated in the GMSL control
> +  channel.
> +
> +  The RDACM20 DT node should be a direct child of the GMSL Deserializer's I2C
> +  bus corresponding to the GMSL link that the camera is attached to.
> +
> +properties:
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0

Are those two properties needed ?

> +
> +  compatible:
> +    const: imi,rdacm20
> +
> +  reg:
> +    description: -|
> +      I2C device addresses, the first to be assigned to the serializer the
> +      second to be assigned to the camera sensor. An optional third address can
> +      be provided to specify the MCU address if present.
> +    minItems: 2
> +    maxItems: 3
> +
> +  port:
> +    type: object
> +    additionalProperties: false
> +    description: -|
> +      Connection to the remote GMSL endpoint are modelled using the OF graph
> +      bindings in accordance with the video interface bindings defined in
> +      Documentation/devicetree/bindings/media/video-interfaces.txt.
> +
> +      The device node contains a single "port" child node with a single
> +      "endpoint" sub-device.
> +
> +    properties:
> +      endpoint:
> +        type: object
> +        additionalProperties: false
> +
> +        properties:
> +          remote-endpoint:
> +            description: -|
> +              phandle to the remote GMSL endpoint sub-node in the remote node
> +              port.
> +            maxItems: 1
> +
> +        required:
> +          - remote-endpoint
> +
> +    required:
> +      - endpoint
> +
> +required:
> +  - compatible
> +  - reg
> +  - port
> +
> +examples:
> +  - |
> +    i2c@e66d8000 {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      reg = <0 0xe66d8000 0 0x40>;
> +
> +      camera@31 {
> +        compatible = "imi,rdacm20";
> +        reg = <0x31>, <0x41>, <0x51>;
> +
> +        port {
> +          rdacm20_out0: endpoint {
> +            remote-endpoint = <&max9286_in0>;
> +          };
> +        };
> +      };
> +    };

-- 
Regards,

Laurent Pinchart

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

* Re: [RFC 05/11] fixup! dt-bindings: media: i2c: Add bindings for IMI RDACM20
  2019-12-16 22:42   ` Laurent Pinchart
@ 2020-02-14  9:07     ` Kieran Bingham
  0 siblings, 0 replies; 18+ messages in thread
From: Kieran Bingham @ 2020-02-14  9:07 UTC (permalink / raw)
  To: Laurent Pinchart, Jacopo Mondi; +Cc: niklas.soderlund, linux-renesas-soc

Hi Jacopo,


On 16/12/2019 22:42, Laurent Pinchart wrote:
> Hi Jacopo,
> 
> Thank you for the patch.
> 
> On Mon, Dec 16, 2019 at 06:16:14PM +0100, Jacopo Mondi wrote:
>> ---
>>  .../bindings/media/i2c/imi,rdacm20.txt        |  66 ----------
>>  .../bindings/media/i2c/imi,rdacm20.yaml       | 113 ++++++++++++++++++
>>  2 files changed, 113 insertions(+), 66 deletions(-)
>>  delete mode 100644 Documentation/devicetree/bindings/media/i2c/imi,rdacm20.txt
>>  create mode 100644 Documentation/devicetree/bindings/media/i2c/imi,rdacm20.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/imi,rdacm20.txt b/Documentation/devicetree/bindings/media/i2c/imi,rdacm20.txt
>> deleted file mode 100644
>> index 4731aafed63f..000000000000
>> --- a/Documentation/devicetree/bindings/media/i2c/imi,rdacm20.txt
>> +++ /dev/null
>> @@ -1,66 +0,0 @@
>> -IMI D&D RDACM20 Automotive Camera Platform
>> -------------------------------------------
>> -
>> -The IMI D&D RDACM20 is a GMSL-compatible camera designed for automotive
>> -applications. It encloses a Maxim Integrated MAX9271 GMSL serializer, an
>> -Omnivision OV10635 camera sensor and an embedded MCU, and connects to a remote
>> -GMSL endpoint through a coaxial cable.
>> -
>> -                                                     IMI RDACM20
>> - ---------------                               --------------------------------
>> -|      GMSL     |   <---  Video Stream        |       <- Video--------\        |
>> -|               |< ====== GMSL Link ======== >|MAX9271<- I2C bus-> <-->OV10635 |
>> -| de-serializer |   <---  I2C messages --->   |                   \<-->MCU     |
>> - ---------------                               --------------------------------
>> -
>> -The RDACM20 transmits video data generated by the embedded camera sensor on the
>> -GMSL serial channel to a remote GMSL de-serializer, as well as it receives and
>> -transmits I2C messages encapsulated in the GMSL bidirectional control channel.
>> -
>> -All I2C traffic received on the GMSL link not directed to the serializer is
>> -propagated on the local I2C bus to the embedded camera sensor and MCU. All
>> -I2C traffic generated on the local I2C bus not directed to the serializer is
>> -propagated to the remote de-serializer encapsulated in the GMSL control channel.
>> -
>> -The RDACM20 DT node should be a direct child of the GMSL Deserializer's I2C bus
>> -corresponding to the GMSL link that the camera is attached to.
>> -
>> -Required Properties:
>> -
>> -- compatible: Shall be "imi,rdacm20".
>> -- reg: I2C device addresses, the first to be assigned to the serializer
>> -  the second to be assigned to the camera sensor. An optional third address can
>> -  be provided to specify the MCU address if present.
>> -
>> -Connection to the remote GMSL endpoint are modelled using the OF graph bindings
>> -in accordance with the video interface bindings defined in
>> -Documentation/devicetree/bindings/media/video-interfaces.txt.
>> -
>> -The device node contains a single "port" child node with a single "endpoint"
>> -sub-device.
>> -
>> -Required endpoint properties:
>> -
>> -- remote-endpoint: phandle to the remote GMSL endpoint sub-node in the remote
>> -  node port.
>> -
>> -Example:
>> --------
>> -
>> -	i2c@0 {
>> -		#address-cells = <1>;
>> -		#size-cells = <0>;
>> -		reg = <0>;
>> -
>> -		camera@51 {
>> -			compatible = "imi,rdacm20";
>> -			reg = <0x31 0x41 0x51>;
>> -
>> -			port {
>> -				rdacm20_out0: endpoint {
>> -					remote-endpoint = <&max9286_in0>;
>> -				};
>> -			};
>> -
>> -		};
>> -	};
>> diff --git a/Documentation/devicetree/bindings/media/i2c/imi,rdacm20.yaml b/Documentation/devicetree/bindings/media/i2c/imi,rdacm20.yaml
>> new file mode 100644
>> index 000000000000..76740e285f44
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/i2c/imi,rdacm20.yaml
>> @@ -0,0 +1,113 @@
>> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
>> +# Copyright (C) 2019 Renesas Electronics Corp.
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/media/i2c/imi,rdacm20.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title:  IMI D&D RDACM20 Automotive Camera Platform
>> +
>> +maintainers:
>> +  - Jacopo Mondi <jacopo+renesas@jmondi.org>
>> +  - Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>> +  - Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>> +  - Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
>> +
>> +description: -|
>> +  The IMI D&D RDACM20 is a GMSL-compatible camera designed for automotive
>> +  applications. It encloses a Maxim Integrated MAX9271 GMSL serializer, an
>> +  Omnivision OV10635 camera sensor and an embedded MCU, and connects to a remote
>> +  GMSL endpoint through a coaxial cable.
>> +
>> +                                                   IMI RDACM20
>> +  +---------------+                        +--------------------------------+
>> +  |      GMSL     |   <- Video Stream      |       <- Video--------\        |
>> +  |               |< === GMSL Link ====== >|MAX9271<- I2C bus-> <-->OV10635 |
>> +  | de-serializer |   <- I2C messages ->   |                   \<-->MCU     |
>> +  +---------------+                        +--------------------------------+
>> +
>> +  The RDACM20 transmits video data generated by the embedded camera sensor on
>> +  the GMSL serial channel to a remote GMSL de-serializer, as well as it receives
>> +  and transmits I2C messages encapsulated in the GMSL bidirectional control
>> +  channel.
>> +
>> +  All I2C traffic received on the GMSL link not directed to the serializer is
>> +  propagated on the local I2C bus to the embedded camera sensor and MCU. All I2C
>> +  traffic generated on the local I2C bus not directed to the serializer is
>> +  propagated to the remote de-serializer encapsulated in the GMSL control
>> +  channel.
>> +
>> +  The RDACM20 DT node should be a direct child of the GMSL Deserializer's I2C
>> +  bus corresponding to the GMSL link that the camera is attached to.
>> +
>> +properties:
>> +  '#address-cells':
>> +    const: 1
>> +
>> +  '#size-cells':
>> +    const: 0
> 
> Are those two properties needed ?
> 

Hi Jacopo,

This question is left unanswered, and the properties are still in the
code base...

Can/should we drop these two properties?


I can see that the i2c-node will specify the properties, but I don't
think the rdacm20 node will..

i.e.:

>> +    i2c@e66d8000 {
>> +      #address-cells = <1>;
>> +      #size-cells = <0>;

	They are utilised here (in the i2c node)...

>> +
>> +      reg = <0 0xe66d8000 0 0x40>;
>> +
>> +      camera@31 {
>> +        compatible = "imi,rdacm20";
>> +        reg = <0x31>, <0x41>, <0x51>;

but not here in the camera node....

>> +
>> +        port {
>> +          rdacm20_out0: endpoint {
>> +            remote-endpoint = <&max9286_in0>;
>> +          };
>> +        };
>> +      };
>> +    };


--
Kieran


>> +
>> +  compatible:
>> +    const: imi,rdacm20
>> +
>> +  reg:
>> +    description: -|
>> +      I2C device addresses, the first to be assigned to the serializer the
>> +      second to be assigned to the camera sensor. An optional third address can
>> +      be provided to specify the MCU address if present.
>> +    minItems: 2
>> +    maxItems: 3
>> +
>> +  port:
>> +    type: object
>> +    additionalProperties: false
>> +    description: -|
>> +      Connection to the remote GMSL endpoint are modelled using the OF graph
>> +      bindings in accordance with the video interface bindings defined in
>> +      Documentation/devicetree/bindings/media/video-interfaces.txt.
>> +
>> +      The device node contains a single "port" child node with a single
>> +      "endpoint" sub-device.
>> +
>> +    properties:
>> +      endpoint:
>> +        type: object
>> +        additionalProperties: false
>> +
>> +        properties:
>> +          remote-endpoint:
>> +            description: -|
>> +              phandle to the remote GMSL endpoint sub-node in the remote node
>> +              port.
>> +            maxItems: 1
>> +
>> +        required:
>> +          - remote-endpoint
>> +
>> +    required:
>> +      - endpoint
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - port
>> +
>> +examples:
>> +  - |
>> +    i2c@e66d8000 {
>> +      #address-cells = <1>;
>> +      #size-cells = <0>;
>> +
>> +      reg = <0 0xe66d8000 0 0x40>;
>> +
>> +      camera@31 {
>> +        compatible = "imi,rdacm20";
>> +        reg = <0x31>, <0x41>, <0x51>;
>> +
>> +        port {
>> +          rdacm20_out0: endpoint {
>> +            remote-endpoint = <&max9286_in0>;
>> +          };
>> +        };
>> +      };
>> +    };
> 


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

end of thread, other threads:[~2020-02-14  9:07 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-16 17:16 [RFC 00/11] GMSL: Initial RDACM21 support Jacopo Mondi
2019-12-16 17:16 ` [RFC 01/11] fixup! DNI: Debug Jacopo Mondi
2019-12-16 22:34   ` Kieran Bingham
2019-12-16 17:16 ` [RFC 02/11] fixup! arm64: dts: renesas: salvator-x: Add MAX9286 expansion board Jacopo Mondi
2019-12-16 22:37   ` Kieran Bingham
2019-12-16 17:16 ` [RFC 03/11] fixup! arm64: dts: renesas: eagle: Provide Eagle FAKRA dynamic overlay Jacopo Mondi
2019-12-16 22:40   ` Kieran Bingham
2019-12-16 17:16 ` [RFC 04/11] fixup! arm64: dts: renesas: eagle: Provide MAX9286 GMSL deserialiser Jacopo Mondi
2019-12-16 22:41   ` Kieran Bingham
2019-12-16 17:16 ` [RFC 05/11] fixup! dt-bindings: media: i2c: Add bindings for IMI RDACM20 Jacopo Mondi
2019-12-16 22:42   ` Laurent Pinchart
2020-02-14  9:07     ` Kieran Bingham
2019-12-16 17:16 ` [RFC 06/11] media: i2c: Break out max9271 from rdacm20 driver Jacopo Mondi
2019-12-16 17:16 ` [RFC 07/11] media: i2c: max9286: Move notifiers operations Jacopo Mondi
2019-12-16 17:16 ` [RFC 08/11] media: i2c: max9286: Move link setup to completion Jacopo Mondi
2019-12-16 17:16 ` [RFC 09/11] media: i2c: max9286: Expand reverse chanenl amplitude Jacopo Mondi
2019-12-16 17:16 ` [RFC 10/11] WIP: media: i2c: rdacm20: Add RDACM21 support Jacopo Mondi
2019-12-16 17:16 ` [RFC 11/11] arm64: boot: dts: Eagle: Enable RDACM21 Jacopo Mondi

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).