[PATCH v2 0/4] media: omap3isp/et8ek8: Add CCP2 CRC configuration support

[PATCH v2 0/4] media: omap3isp/et8ek8: Add CCP2 CRC configuration support

[Alex Tran]

This patch series provides support for configuring CRC checksum via device tree
for et8ek8 sensor and OMAP3 ISP receiver.

Previously, CRC was hardcoded to enabled (1) in the sensor driver and ISP
receiver. This series makes it configurable through DT, allowing both
sides to be aligned, as both must use the same CRC setting for proper CCP2
communication.

Changes maintain backward compatibility by defaulting CRC to 1 when the
property is not specified in the device tree.

The series also converts both device tree bindings from TXT to YAML
schema format, as required.

Changes in v2:
- Fixed broken patch threading
- Added receiver support for reading crc from device tree
- Converted both sensor and ISP bindings from TXT to YAML format
- Both sensor and ISP endpoints can now be configured consistently via DT

Alex Tran (4):
  media: i2c: et8ek8: et8ek8_driver: add support for crc configuration
    via device tree
  dt-bindings: media: i2c: et8ek8: document missing crc as optional
    property
  media: platform: ti: omap3isp: isp: read crc configuration from device
    tree for CCP2
  dt-bindings: media: omap3isp: document missing crc as optional
    property

 .../bindings/media/i2c/toshiba,et8ek8.txt     |  55 -----
 .../bindings/media/i2c/toshiba,et8ek8.yaml    |  99 +++++++++
 .../devicetree/bindings/media/ti,omap3isp.txt |  71 -------
 .../bindings/media/ti,omap3isp.yaml           | 196 ++++++++++++++++++
 MAINTAINERS                                   |   1 +
 drivers/media/i2c/et8ek8/et8ek8_driver.c      |  49 ++++-
 drivers/media/platform/ti/omap3isp/isp.c      |   5 +-
 7 files changed, 339 insertions(+), 137 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/media/i2c/toshiba,et8ek8.txt
 create mode 100644 Documentation/devicetree/bindings/media/i2c/toshiba,et8ek8.yaml
 delete mode 100644 Documentation/devicetree/bindings/media/ti,omap3isp.txt
 create mode 100644 Documentation/devicetree/bindings/media/ti,omap3isp.yaml

-- 
2.51.0

[PATCH v2 1/4] media: i2c: et8ek8: et8ek8_driver: add support for crc configuration via device tree

[Alex Tran]

Retrieve the configuration for CRC from the device tree instead
using the hard coded USE_CRC macro. If there is an issue
retrieving the endpoint node or the CRC property then the default
of 1 is used and the driver does not fail to maintain backward
compatibility.

Signed-off-by: Alex Tran <alex.t.tran@gmail.com>
---
 drivers/media/i2c/et8ek8/et8ek8_driver.c | 49 +++++++++++++++++++-----
 1 file changed, 39 insertions(+), 10 deletions(-)

diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c b/drivers/media/i2c/et8ek8/et8ek8_driver.c
index 2cb7b7187..4ef92359c 100644
--- a/drivers/media/i2c/et8ek8/et8ek8_driver.c
+++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c
@@ -29,6 +29,7 @@
 #include <media/media-entity.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
+#include <media/v4l2-fwnode.h>
 #include <media/v4l2-subdev.h>
 
 #include "et8ek8_reg.h"
@@ -45,6 +46,7 @@ struct et8ek8_sensor {
 	struct regulator *vana;
 	struct clk *ext_clk;
 	u32 xclk_freq;
+	u32 use_crc;
 
 	u16 version;
 
@@ -130,8 +132,6 @@ static struct et8ek8_gain {
 
 #define ET8EK8_I2C_DELAY	3	/* msec delay b/w accesses */
 
-#define USE_CRC			1
-
 /*
  * Register access helpers
  *
@@ -844,20 +844,16 @@ static int et8ek8_power_on(struct et8ek8_sensor *sensor)
 	if (rval)
 		goto out;
 
-#ifdef USE_CRC
 	rval = et8ek8_i2c_read_reg(client, ET8EK8_REG_8BIT, 0x1263, &val);
 	if (rval)
 		goto out;
-#if USE_CRC /* TODO get crc setting from DT */
-	val |= BIT(4);
-#else
-	val &= ~BIT(4);
-#endif
+	if (sensor->use_crc)
+		val |= BIT(4);
+	else
+		val &= ~BIT(4);
 	rval = et8ek8_i2c_write_reg(client, ET8EK8_REG_8BIT, 0x1263, val);
 	if (rval)
 		goto out;
-#endif
-
 out:
 	if (rval)
 		et8ek8_power_off(sensor);
@@ -1396,6 +1392,34 @@ static int __maybe_unused et8ek8_resume(struct device *dev)
 	return __et8ek8_set_power(sensor, true);
 }
 
+static int et8ek8_parse_fwnode(struct device *dev, struct et8ek8_sensor *sensor)
+{
+	struct v4l2_fwnode_endpoint bus_cfg = {
+		.bus_type = V4L2_MBUS_CCP2,
+	};
+	struct fwnode_handle *ep;
+	int ret;
+
+	ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 0, 0,
+					     FWNODE_GRAPH_ENDPOINT_NEXT);
+	if (!ep) {
+		dev_warn(dev, "could not get endpoint node\n");
+		return -EINVAL;
+	}
+
+	ret = v4l2_fwnode_endpoint_alloc_parse(ep, &bus_cfg);
+	if (ret) {
+		dev_warn(dev, "parsing endpoint node failed\n");
+		goto done;
+	}
+
+	fwnode_property_read_u32(ep, "crc", &sensor->use_crc);
+done:
+	v4l2_fwnode_endpoint_free(&bus_cfg);
+	fwnode_handle_put(ep);
+	return ret;
+}
+
 static int et8ek8_probe(struct i2c_client *client)
 {
 	struct et8ek8_sensor *sensor;
@@ -1406,6 +1430,11 @@ static int et8ek8_probe(struct i2c_client *client)
 	if (!sensor)
 		return -ENOMEM;
 
+	sensor->use_crc = 1;
+	ret = et8ek8_parse_fwnode(dev, sensor);
+	if (ret)
+		dev_warn(dev, "parsing endpoint failed\n");
+
 	sensor->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
 	if (IS_ERR(sensor->reset)) {
 		dev_dbg(&client->dev, "could not request reset gpio\n");
-- 
2.51.0

[PATCH v2 2/4] dt-bindings: media: i2c: et8ek8: document missing crc as optional property

[Alex Tran]

Convert the et8ek8 sensor device tree binding from TXT to YAML format.
Add the optional crc property to the endpoint node for the et8ek8 sensor.
This property enables CRC checksums for the sensor bus and was added to
match the new driver support for reading it from the device tree.
Add documentation reference to MAINTAINERS file.

Signed-off-by: Alex Tran <alex.t.tran@gmail.com>
---
 .../bindings/media/i2c/toshiba,et8ek8.txt     | 55 -----------
 .../bindings/media/i2c/toshiba,et8ek8.yaml    | 99 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 3 files changed, 100 insertions(+), 55 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/media/i2c/toshiba,et8ek8.txt
 create mode 100644 Documentation/devicetree/bindings/media/i2c/toshiba,et8ek8.yaml

diff --git a/Documentation/devicetree/bindings/media/i2c/toshiba,et8ek8.txt b/Documentation/devicetree/bindings/media/i2c/toshiba,et8ek8.txt
deleted file mode 100644
index 8d8e40c56..000000000
--- a/Documentation/devicetree/bindings/media/i2c/toshiba,et8ek8.txt
+++ /dev/null
@@ -1,55 +0,0 @@
-Toshiba et8ek8 5MP sensor
-
-Toshiba et8ek8 5MP sensor is an image sensor found in Nokia N900 device
-
-More detailed documentation can be found in
-Documentation/devicetree/bindings/media/video-interfaces.txt .
-
-
-Mandatory properties
---------------------
-
-- compatible: "toshiba,et8ek8"
-- reg: I2C address (0x3e, or an alternative address)
-- vana-supply: Analogue voltage supply (VANA), 2.8 volts
-- clocks: External clock to the sensor
-- reset-gpios: XSHUTDOWN GPIO. The XSHUTDOWN signal is active low. The sensor
-  is in hardware standby mode when the signal is in the low state.
-
-
-Optional properties
--------------------
-
-- flash-leds: See ../video-interfaces.txt
-- lens-focus: See ../video-interfaces.txt
-
-
-Endpoint node mandatory properties
-----------------------------------
-
-- remote-endpoint: A phandle to the bus receiver's endpoint node.
-
-
-Example
--------
-
-&i2c3 {
-	clock-frequency = <400000>;
-
-	cam1: camera@3e {
-		compatible = "toshiba,et8ek8";
-		reg = <0x3e>;
-		vana-supply = <&vaux4>;
-
-		clocks = <&isp 0>;
-		assigned-clocks = <&isp 0>;
-		assigned-clock-rates = <9600000>;
-
-		reset-gpio = <&gpio4 6 GPIO_ACTIVE_HIGH>; /* 102 */
-		port {
-			csi_cam1: endpoint {
-				remote-endpoint = <&csi_out1>;
-			};
-		};
-	};
-};
diff --git a/Documentation/devicetree/bindings/media/i2c/toshiba,et8ek8.yaml b/Documentation/devicetree/bindings/media/i2c/toshiba,et8ek8.yaml
new file mode 100644
index 000000000..6e7f60ee9
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/toshiba,et8ek8.yaml
@@ -0,0 +1,99 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/toshiba,et8ek8.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Toshiba et8ek8 5MP sensor
+
+maintainers:
+  - Pavel Machek <pavel@ucw.cz>
+  - Sakari Ailus <sakari.ailus@iki.fi>
+
+description:
+  Toshiba et8ek8 5MP sensor is an image sensor found in Nokia N900 device
+
+properties:
+  compatible:
+    const: toshiba,et8ek8
+
+  reg:
+    description:
+      I2C address (0x3e, or an alternative address)
+    maxItems: 1
+
+  vana-supply:
+    description:
+      Analogue voltage supply (VANA), 2.8 volts
+
+  clocks:
+    description:
+      External clock to the sensor
+    maxItems: 1
+
+  reset-gpios:
+    description:
+      XSHUTDOWN GPIO. The XSHUTDOWN signal is active low. The sensor
+      is in hardware standby mode when the signal is in the low state.
+    maxItems: 1
+
+  flash-leds:
+    $ref: /schemas/media/video-interfaces.yaml#
+
+  lens-focus:
+    $ref: /schemas/media/video-interfaces.yaml#
+
+  port:
+    $ref: /schemas/graph.yaml#/$defs/port-base
+    additionalProperties: false
+
+    properties:
+      endpoint:
+        $ref: /schemas/media/video-interfaces.yaml#
+        unevaluatedProperties: false
+
+        properties:
+          crc:
+            $ref: /schemas/types.yaml#/definitions/uint32
+            enum: [0, 1]
+            default: 1
+            description:
+              Enable CRC checksums.
+
+          remote-endpoint: true
+
+required:
+  - compatible
+  - reg
+  - vana-supply
+  - clocks
+  - reset-gpios
+  - port
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        camera@3e {
+            compatible = "toshiba,et8ek8";
+            reg = <0x3e>;
+            vana-supply = <&vaux4>;
+            clocks = <&isp 0>;
+            assigned-clocks = <&isp 0>;
+            assigned-clock-rates = <9600000>;
+            reset-gpios = <&gpio4 6 GPIO_ACTIVE_HIGH>;
+
+            port {
+                csi_cam1: endpoint {
+                    remote-endpoint = <&csi_out1>;
+                    crc = <1>;
+                };
+            };
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 8aec054a6..6a9ce4d17 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18308,6 +18308,7 @@ M:	Sakari Ailus <sakari.ailus@iki.fi>
 L:	linux-media@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/media/i2c/adi,ad5820.yaml
+F:	Documentation/devicetree/bindings/media/i2c/toshiba,et8ek8.yaml
 F:	drivers/media/i2c/ad5820.c
 F:	drivers/media/i2c/et8ek8
 
-- 
2.51.0

[PATCH v2 3/4] media: platform: ti: omap3isp: isp: read crc configuration from device tree for CCP2

[Alex Tran]

Allow CCP2 receiver to read the crc configuration from the device tree.
Default value of 1 is used to maintain backward compatibility with
existing device trees that don't specify the crc property.

Signed-off-by: Alex Tran <alex.t.tran@gmail.com>
---
 drivers/media/platform/ti/omap3isp/isp.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/ti/omap3isp/isp.c b/drivers/media/platform/ti/omap3isp/isp.c
index 8ac2bdcdf..cd10589ed 100644
--- a/drivers/media/platform/ti/omap3isp/isp.c
+++ b/drivers/media/platform/ti/omap3isp/isp.c
@@ -2102,7 +2102,10 @@ static void isp_parse_of_csi1_endpoint(struct device *dev,
 	buscfg->bus.ccp2.ccp2_mode = vep->bus_type == V4L2_MBUS_CCP2;
 	buscfg->bus.ccp2.vp_clk_pol = 1;
 
-	buscfg->bus.ccp2.crc = 1;
+	u32 use_crc = 1;
+
+	fwnode_property_read_u32(vep->base.local_fwnode, "crc", &use_crc);
+	buscfg->bus.ccp2.crc = use_crc;
 }
 
 static struct {
-- 
2.51.0

[PATCH v2 4/4] dt-bindings: media: omap3isp: document missing crc as optional property

[Alex Tran]

Convert the OMAP3 ISP device tree binding from TXT format to YAML.
Add the optional crc property to the endpoint node for the omap3isp
receiver. This property enables CRC checksums for the bus and was added to
match the new driver support for reading it from the device tree.

Signed-off-by: Alex Tran <alex.t.tran@gmail.com>
---
 .../devicetree/bindings/media/ti,omap3isp.txt |  71 -------
 .../bindings/media/ti,omap3isp.yaml           | 196 ++++++++++++++++++
 2 files changed, 196 insertions(+), 71 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/media/ti,omap3isp.txt
 create mode 100644 Documentation/devicetree/bindings/media/ti,omap3isp.yaml

diff --git a/Documentation/devicetree/bindings/media/ti,omap3isp.txt b/Documentation/devicetree/bindings/media/ti,omap3isp.txt
deleted file mode 100644
index ac23de855..000000000
--- a/Documentation/devicetree/bindings/media/ti,omap3isp.txt
+++ /dev/null
@@ -1,71 +0,0 @@
-OMAP 3 ISP Device Tree bindings
-===============================
-
-The DT definitions can be found in include/dt-bindings/media/omap3-isp.h.
-
-Required properties
-===================
-
-compatible	: must contain "ti,omap3-isp"
-
-reg		: the two registers sets (physical address and length) for the
-		  ISP. The first set contains the core ISP registers up to
-		  the end of the SBL block. The second set contains the
-		  CSI PHYs and receivers registers.
-interrupts	: the ISP interrupt specifier
-iommus		: phandle and IOMMU specifier for the IOMMU that serves the ISP
-syscon		: the phandle and register offset to the Complex I/O or CSI-PHY
-		  register
-ti,phy-type	: 0 -- OMAP3ISP_PHY_TYPE_COMPLEX_IO (e.g. 3430)
-		  1 -- OMAP3ISP_PHY_TYPE_CSIPHY (e.g. 3630)
-#clock-cells	: Must be 1 --- the ISP provides two external clocks,
-		  cam_xclka and cam_xclkb, at indices 0 and 1,
-		  respectively. Please find more information on common
-		  clock bindings in ../clock/clock-bindings.txt.
-
-Port nodes (optional)
----------------------
-
-More documentation on these bindings is available in
-video-interfaces.txt in the same directory.
-
-reg		: The interface:
-		  0 - parallel (CCDC)
-		  1 - CSIPHY1 -- CSI2C / CCP2B on 3630;
-		      CSI1 -- CSIb on 3430
-		  2 - CSIPHY2 -- CSI2A / CCP2B on 3630;
-		      CSI2 -- CSIa on 3430
-
-Optional properties
-===================
-
-vdd-csiphy1-supply : voltage supply of the CSI-2 PHY 1
-vdd-csiphy2-supply : voltage supply of the CSI-2 PHY 2
-
-Endpoint nodes
---------------
-
-lane-polarities	: lane polarity (required on CSI-2)
-		  0 -- not inverted; 1 -- inverted
-data-lanes	: an array of data lanes from 1 to 3. The length can
-		  be either 1 or 2. (required on CSI-2)
-clock-lanes	: the clock lane (from 1 to 3). (required on CSI-2)
-
-
-Example
-=======
-
-		isp@480bc000 {
-			compatible = "ti,omap3-isp";
-			reg = <0x480bc000 0x12fc
-			       0x480bd800 0x0600>;
-			interrupts = <24>;
-			iommus = <&mmu_isp>;
-			syscon = <&scm_conf 0x2f0>;
-			ti,phy-type = <OMAP3ISP_PHY_TYPE_CSIPHY>;
-			#clock-cells = <1>;
-			ports {
-				#address-cells = <1>;
-				#size-cells = <0>;
-			};
-		};
diff --git a/Documentation/devicetree/bindings/media/ti,omap3isp.yaml b/Documentation/devicetree/bindings/media/ti,omap3isp.yaml
new file mode 100644
index 000000000..b86c3aa71
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/ti,omap3isp.yaml
@@ -0,0 +1,196 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/ti,omap3isp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments OMAP 3 Image Signal Processor (ISP)
+
+maintainers:
+  - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+  - Sakari Ailus <sakari.ailus@iki.fi>
+
+description:
+  The OMAP 3 ISP is an image signal processor present in OMAP 3 SoCs.
+
+properties:
+  compatible:
+    const: ti,omap3-isp
+
+  reg:
+    items:
+      - description: Core ISP registers up to the end of the SBL block
+      - description: CSI PHYs and receivers registers
+
+  interrupts:
+    maxItems: 1
+    description: the ISP interrupt specifier
+
+  iommus:
+    maxItems: 1
+    description: phandle and IOMMU specifier for the IOMMU that serves the ISP
+
+  syscon:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      - items:
+          - description: phandle to System Control Module
+          - description: register offset to Complex I/O or CSI-PHY register
+    description:
+      Phandle and register offset to the Complex I/O or CSI-PHY register
+
+  ti,phy-type:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1]
+    description:
+      0 - OMAP3ISP_PHY_TYPE_COMPLEX_IO (e.g. OMAP 3430)
+      1 - OMAP3ISP_PHY_TYPE_CSIPHY (e.g. OMAP 3630)
+
+  '#clock-cells':
+    const: 1
+    description:
+      The ISP provides two external clocks, cam_xclka and cam_xclkb,
+      at indices 0 and 1 respectively.
+
+  vdd-csiphy1-supply:
+    description: Voltage supply of the CSI-2 PHY 1
+
+  vdd-csiphy2-supply:
+    description: Voltage supply of the CSI-2 PHY 2
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        unevaluatedProperties: false
+        description: Parallel (CCDC) interface
+
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+            unevaluatedProperties: false
+
+      port@1:
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        unevaluatedProperties: false
+        description: |
+          CSIPHY1 interface:
+            OMAP 3630: CSI2C / CCP2B
+            OMAP 3430: CSI1 (CSIb)
+
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+            unevaluatedProperties: false
+
+            properties:
+              lane-polarities:
+                description: |
+                  The lane polarity (required on CSI-2):
+                    0 - not inverted
+                    1 - inverted
+                minItems: 2
+                maxItems: 3
+                items:
+                  enum: [0, 1]
+
+              data-lanes:
+                description: Data lanes (required on CSI-2)
+                minItems: 1
+                maxItems: 2
+                items:
+                  minimum: 1
+                  maximum: 3
+
+              clock-lanes:
+                description: The clock lane (required on CSI-2)
+                maxItems: 1
+                items:
+                  minimum: 1
+                  maximum: 3
+
+              crc:
+                $ref: /schemas/types.yaml#/definitions/uint32
+                enum: [0, 1]
+                default: 1
+                description:
+                  Enable CRC checksums.
+
+      port@2:
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        unevaluatedProperties: false
+        description: |
+          CSIPHY2 interface:
+            OMAP 3630: CSI2A / CCP2B
+            OMAP 3430: CSI2 (CSIa)
+
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+            unevaluatedProperties: false
+
+            properties:
+              lane-polarities:
+                description: |
+                  The lane polarity (required on CSI-2):
+                    0 - not inverted
+                    1 - inverted
+                minItems: 2
+                maxItems: 3
+                items:
+                  enum: [0, 1]
+
+              data-lanes:
+                description: Data lanes (required on CSI-2)
+                minItems: 1
+                maxItems: 2
+                items:
+                  minimum: 1
+                  maximum: 3
+
+              clock-lanes:
+                description: The clock lane (required on CSI-2)
+                maxItems: 1
+                items:
+                  minimum: 1
+                  maximum: 3
+
+              crc:
+                $ref: /schemas/types.yaml#/definitions/uint32
+                enum: [0, 1]
+                default: 1
+                description:
+                  Enable CRC checksums.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - iommus
+  - syscon
+  - ti,phy-type
+  - '#clock-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/media/omap3-isp.h>
+
+    isp@480bc000 {
+        compatible = "ti,omap3-isp";
+        reg = <0x480bc000 0x12fc>,
+              <0x480bd800 0x0600>;
+        interrupts = <24>;
+        iommus = <&mmu_isp>;
+        syscon = <&scm_conf 0x2f0>;
+        ti,phy-type = <OMAP3ISP_PHY_TYPE_CSIPHY>;
+        #clock-cells = <1>;
+
+        ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+        };
+    };
-- 
2.51.0

Re: [PATCH v2 2/4] dt-bindings: media: i2c: et8ek8: document missing crc as optional property

[Krzysztof Kozlowski]

On Sun, Dec 14, 2025 at 11:58:33PM -0800, Alex Tran wrote:
> Convert the et8ek8 sensor device tree binding from TXT to YAML format.

Commit subject is completely wrong. You are not documenting missing
properties.

> Add the optional crc property to the endpoint node for the et8ek8 sensor.

Why?

> This property enables CRC checksums for the sensor bus and was added to
> match the new driver support for reading it from the device tree.

This does not explain me why.

I asked you first to convert the bindings, not add new properties. If
you add new properties, you must provide justification why binding has
to be changed - see other commits how to do this. Also please read
submitting patches how to split your work into logical chunks.

Best regards,
Krzysztof

Re: [PATCH v2 1/4] media: i2c: et8ek8: et8ek8_driver: add support for crc configuration via device tree

[Krzysztof Kozlowski]

On Sun, Dec 14, 2025 at 11:58:32PM -0800, Alex Tran wrote:
> Retrieve the configuration for CRC from the device tree instead
> using the hard coded USE_CRC macro. If there is an issue
> retrieving the endpoint node or the CRC property then the default
> of 1 is used and the driver does not fail to maintain backward
> compatibility.
> 
> Signed-off-by: Alex Tran <alex.t.tran@gmail.com>
> ---
>  drivers/media/i2c/et8ek8/et8ek8_driver.c | 49 +++++++++++++++++++-----
>  1 file changed, 39 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c b/drivers/media/i2c/et8ek8/et8ek8_driver.c
> index 2cb7b7187..4ef92359c 100644
> --- a/drivers/media/i2c/et8ek8/et8ek8_driver.c
> +++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c
> @@ -29,6 +29,7 @@
>  #include <media/media-entity.h>
>  #include <media/v4l2-ctrls.h>
>  #include <media/v4l2-device.h>
> +#include <media/v4l2-fwnode.h>
>  #include <media/v4l2-subdev.h>
>  
>  #include "et8ek8_reg.h"
> @@ -45,6 +46,7 @@ struct et8ek8_sensor {
>  	struct regulator *vana;
>  	struct clk *ext_clk;
>  	u32 xclk_freq;
> +	u32 use_crc;
>  
>  	u16 version;
>  
> @@ -130,8 +132,6 @@ static struct et8ek8_gain {
>  
>  #define ET8EK8_I2C_DELAY	3	/* msec delay b/w accesses */
>  
> -#define USE_CRC			1
> -
>  /*
>   * Register access helpers
>   *
> @@ -844,20 +844,16 @@ static int et8ek8_power_on(struct et8ek8_sensor *sensor)
>  	if (rval)
>  		goto out;
>  
> -#ifdef USE_CRC
>  	rval = et8ek8_i2c_read_reg(client, ET8EK8_REG_8BIT, 0x1263, &val);
>  	if (rval)
>  		goto out;
> -#if USE_CRC /* TODO get crc setting from DT */
> -	val |= BIT(4);
> -#else
> -	val &= ~BIT(4);
> -#endif
> +	if (sensor->use_crc)
> +		val |= BIT(4);
> +	else
> +		val &= ~BIT(4);
>  	rval = et8ek8_i2c_write_reg(client, ET8EK8_REG_8BIT, 0x1263, val);
>  	if (rval)
>  		goto out;
> -#endif
> -
>  out:
>  	if (rval)
>  		et8ek8_power_off(sensor);
> @@ -1396,6 +1392,34 @@ static int __maybe_unused et8ek8_resume(struct device *dev)
>  	return __et8ek8_set_power(sensor, true);
>  }
>  
> +static int et8ek8_parse_fwnode(struct device *dev, struct et8ek8_sensor *sensor)
> +{
> +	struct v4l2_fwnode_endpoint bus_cfg = {
> +		.bus_type = V4L2_MBUS_CCP2,
> +	};
> +	struct fwnode_handle *ep;
> +	int ret;
> +
> +	ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 0, 0,
> +					     FWNODE_GRAPH_ENDPOINT_NEXT);
> +	if (!ep) {
> +		dev_warn(dev, "could not get endpoint node\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = v4l2_fwnode_endpoint_alloc_parse(ep, &bus_cfg);
> +	if (ret) {
> +		dev_warn(dev, "parsing endpoint node failed\n");
> +		goto done;
> +	}
> +
> +	fwnode_property_read_u32(ep, "crc", &sensor->use_crc);

Undocumented ABI. In case you document it in other cases, it is really
unclear, so be sure you read submitting patches in DT.

Best regards,
Krzysztof

Re: [PATCH v2 4/4] dt-bindings: media: omap3isp: document missing crc as optional property

[Krzysztof Kozlowski]

On Sun, Dec 14, 2025 at 11:58:35PM -0800, Alex Tran wrote:
> Convert the OMAP3 ISP device tree binding from TXT format to YAML.

Same problem with the subject.

I asked to convert the bindings first.

Best regards,
Krzysztof

Re: [PATCH v2 2/4] dt-bindings: media: i2c: et8ek8: document missing crc as optional property

[Alex Tran]

On Wed, Dec 17, 2025 at 12:27 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Sun, Dec 14, 2025 at 11:58:33PM -0800, Alex Tran wrote:
> > Convert the et8ek8 sensor device tree binding from TXT to YAML format.
>
> Commit subject is completely wrong. You are not documenting missing
> properties.
>
> > Add the optional crc property to the endpoint node for the et8ek8 sensor.
>
> Why?
>
> > This property enables CRC checksums for the sensor bus and was added to
> > match the new driver support for reading it from the device tree.
>
> This does not explain me why.
>
> I asked you first to convert the bindings, not add new properties. If
> you add new properties, you must provide justification why binding has
> to be changed - see other commits how to do this. Also please read
> submitting patches how to split your work into logical chunks.
>
> Best regards,
> Krzysztof
>
Thanks for the review. I'll send in a separate patch series first,
focusing solely on the conversion of the bindings without any new
properties.

-- 
Yours,
Alex Tran