[RFC 00/12] staging: media: max96712: Add support for streams and multiple sensors

[RFC 00/12] staging: media: max96712: Add support for streams and multiple sensors

[Laurentiu Palcu]

Hi,

This series adds more functionality to the existing max96712 staging
driver allowing multiple sensors to be connected through other
compatible serializers. I tried to split the changes in smaller logical
changes to make them easier to review while not altering the existing
VPG functionality but I could squash all of them together if needed.

The series only supports tunneling mode and uses the first MIPI-CSI
port. Support for more functionality can be added later, if needed.

I sent the set as a RFC because it depends on Sakari's pending internal
pads patch which is needed if we want to have an elegant solution for
allowing the user to switch between streaming from sensors or just
video pattern generation.

Also, the set depends on my previous series which was not yet merged:
https://patchwork.linuxtv.org/project/linux-media/list/?series=14255

Thanks,
Laurentiu

Laurentiu Palcu (11):
  dt-bindings: i2c: maxim,max96712: add a couple of new properties
  staging: media: max96712: convert to using CCI register access helpers
  staging: media: max96712: change DT parsing routine
  staging: media: max96712: add link frequency V4L2 control
  staging: media: max96712: add I2C mux support
  staging: media: max96712: add support for streams
  staging: media: max96712: allow enumerating MBUS codes
  staging: media: max96712: add set_fmt routine
  staging: media: max96712: add gpiochip functionality
  staging: media: max96712: add fsync support
  staging: media: max96712: allow streaming from connected sensors

Sakari Ailus (1):
  media: mc: Add INTERNAL pad flag

 .../bindings/media/i2c/maxim,max96712.yaml    |   45 +
 .../media/mediactl/media-types.rst            |    8 +
 drivers/media/mc/mc-entity.c                  |   10 +-
 drivers/staging/media/max96712/max96712.c     | 1406 +++++++++++++++--
 include/uapi/linux/media.h                    |    1 +
 5 files changed, 1352 insertions(+), 118 deletions(-)

-- 
2.44.1

[RFC 01/12] media: mc: Add INTERNAL pad flag

[Laurentiu Palcu]

From: Sakari Ailus <sakari.ailus@linux.intel.com>

Internal source pads will be used as routing endpoints in V4L2
[GS]_ROUTING IOCTLs, to indicate that the stream begins in the entity.
Internal source pads are pads that have both SINK and INTERNAL flags set.

Also prevent creating links to pads that have been flagged as internal and
initialising SOURCE pads with INTERNAL flag set.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../userspace-api/media/mediactl/media-types.rst       |  8 ++++++++
 drivers/media/mc/mc-entity.c                           | 10 ++++++++--
 include/uapi/linux/media.h                             |  1 +
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/Documentation/userspace-api/media/mediactl/media-types.rst b/Documentation/userspace-api/media/mediactl/media-types.rst
index 6332e8395263b..f55ef055bcf85 100644
--- a/Documentation/userspace-api/media/mediactl/media-types.rst
+++ b/Documentation/userspace-api/media/mediactl/media-types.rst
@@ -361,6 +361,7 @@ Types and flags used to represent the media graph elements
 .. _MEDIA-PAD-FL-SINK:
 .. _MEDIA-PAD-FL-SOURCE:
 .. _MEDIA-PAD-FL-MUST-CONNECT:
+.. _MEDIA-PAD-FL-INTERNAL:
 
 .. flat-table:: Media pad flags
     :header-rows:  0
@@ -381,6 +382,13 @@ Types and flags used to represent the media graph elements
 	  enabled links even when this flag isn't set; the absence of the flag
 	  doesn't imply there is none.
 
+    *  -  ``MEDIA_PAD_FL_INTERNAL``
+       -  The internal flag indicates an internal pad that has no external
+	  connections. Such a pad shall not be connected with a link.
+
+	  The internal flag may currently be present only in a source pad where
+	  it indicates that the :ref:``stream <media-glossary-stream>``
+	  originates from within the entity.
 
 One and only one of ``MEDIA_PAD_FL_SINK`` and ``MEDIA_PAD_FL_SOURCE``
 must be set for every pad.
diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
index 0455909055820..d1feacc608072 100644
--- a/drivers/media/mc/mc-entity.c
+++ b/drivers/media/mc/mc-entity.c
@@ -213,7 +213,9 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
 		iter->index = i++;
 
 		if (hweight32(iter->flags & (MEDIA_PAD_FL_SINK |
-					     MEDIA_PAD_FL_SOURCE)) != 1) {
+					     MEDIA_PAD_FL_SOURCE)) != 1 ||
+		    (iter->flags & MEDIA_PAD_FL_INTERNAL &&
+		     !(iter->flags & MEDIA_PAD_FL_SINK))) {
 			ret = -EINVAL;
 			break;
 		}
@@ -1118,7 +1120,8 @@ int media_get_pad_index(struct media_entity *entity, u32 pad_type,
 
 	for (i = 0; i < entity->num_pads; i++) {
 		if ((entity->pads[i].flags &
-		     (MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_SOURCE)) != pad_type)
+		     (MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_SOURCE |
+		      MEDIA_PAD_FL_INTERNAL)) != pad_type)
 			continue;
 
 		if (entity->pads[i].sig_type == sig_type)
@@ -1148,6 +1151,9 @@ media_create_pad_link(struct media_entity *source, u16 source_pad,
 		return -EINVAL;
 	if (WARN_ON(!(sink->pads[sink_pad].flags & MEDIA_PAD_FL_SINK)))
 		return -EINVAL;
+	if (WARN_ON(source->pads[source_pad].flags & MEDIA_PAD_FL_INTERNAL) ||
+	    WARN_ON(sink->pads[sink_pad].flags & MEDIA_PAD_FL_INTERNAL))
+		return -EINVAL;
 
 	link = media_add_link(&source->links);
 	if (link == NULL)
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 1c80b1d6bbaf3..80cfd12a43fc1 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -208,6 +208,7 @@ struct media_entity_desc {
 #define MEDIA_PAD_FL_SINK			(1U << 0)
 #define MEDIA_PAD_FL_SOURCE			(1U << 1)
 #define MEDIA_PAD_FL_MUST_CONNECT		(1U << 2)
+#define MEDIA_PAD_FL_INTERNAL			(1U << 3)
 
 struct media_pad_desc {
 	__u32 entity;		/* entity ID */
-- 
2.44.1

[RFC 02/12] dt-bindings: i2c: maxim,max96712: add a couple of new properties

[Laurentiu Palcu]

Add new properties for configuring FSYNC functionality and operation
mode, as the chip can support both tunneling and pixel modes.

While at it, add the maxim,max96724 compatible to the bindings since it
was already added in the driver some time back.

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
---
 .../bindings/media/i2c/maxim,max96712.yaml    | 45 +++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml
index 26f85151afbd3..410004f3a032f 100644
--- a/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml
@@ -36,6 +36,48 @@ properties:
 
   enable-gpios: true
 
+  '#gpio-cells':
+    const: 2
+    description: |
+      First cell is the GPIO pin number, second cell is the flags. The GPIO pin
+      number must be in range of [0, 11].
+
+  gpio-controller: true
+
+  maxim,operation-mode:
+    description: |
+      Deserializer mode of operation: 0 - tunneling mode, 1 - pixel mode
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1]
+    default: 0
+
+  maxim,fsync-config:
+    description: |
+      Frame synchronization (FSYNC) is used to align images sent from multiple
+      sources in surround-view applications and is required for concatenation.
+      In FSYNC mode, the deserializer sends a sync signal to each serializer;
+      the serializers then send the signal to the connected sensor.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 2
+    items:
+      - description: |
+          FSYNC mode:
+            0 - off, no FSYNC generation
+            1 - internal, GPIO is not used as input or output
+            2 - master, GPIO pin is used to drive a slave deserializer
+            3 - slave, GPIO pin is used as FSYNC input driven by a master device
+        enum: [0, 1, 2, 3]
+        default: 0
+      - description: |
+          FSYNC TX ID: GPIO ID used for transmitting FSYNC signal
+        minimum: 0
+        maximum: 31
+        default: 0
+      - description: |
+          FSYNC pin: 0 - MFP0, 1 - MFP7. Not used for internal mode.
+        enum: [0, 1]
+        default: 0
+
   ports:
     $ref: /schemas/graph.yaml#/properties/ports
 
@@ -92,6 +134,9 @@ examples:
     #include <dt-bindings/gpio/gpio.h>
     #include <dt-bindings/media/video-interfaces.h>
 
+    maxim,operation-mode = <0>;
+    maxim,fsync-config = <1 0>;
+
     i2c@e6508000 {
             #address-cells = <1>;
             #size-cells = <0>;
-- 
2.44.1

Re: [RFC 02/12] dt-bindings: i2c: maxim,max96712: add a couple of new properties

[Rob Herring]

On Fri, Jan 31, 2025 at 06:33:56PM +0200, Laurentiu Palcu wrote:
> Add new properties for configuring FSYNC functionality and operation
> mode, as the chip can support both tunneling and pixel modes.
> 
> While at it, add the maxim,max96724 compatible to the bindings since it
> was already added in the driver some time back.

I don't see that change.

> 
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> ---
>  .../bindings/media/i2c/maxim,max96712.yaml    | 45 +++++++++++++++++++
>  1 file changed, 45 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml
> index 26f85151afbd3..410004f3a032f 100644
> --- a/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml
> @@ -36,6 +36,48 @@ properties:
>  
>    enable-gpios: true
>  
> +  '#gpio-cells':
> +    const: 2
> +    description: |

Don't need '|' if no formatting.

> +      First cell is the GPIO pin number, second cell is the flags. The GPIO pin
> +      number must be in range of [0, 11].
> +
> +  gpio-controller: true
> +
> +  maxim,operation-mode:
> +    description: |
> +      Deserializer mode of operation: 0 - tunneling mode, 1 - pixel mode
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1]
> +    default: 0
> +
> +  maxim,fsync-config:
> +    description: |
> +      Frame synchronization (FSYNC) is used to align images sent from multiple
> +      sources in surround-view applications and is required for concatenation.
> +      In FSYNC mode, the deserializer sends a sync signal to each serializer;
> +      the serializers then send the signal to the connected sensor.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 2
> +    items:
> +      - description: |
> +          FSYNC mode:
> +            0 - off, no FSYNC generation
> +            1 - internal, GPIO is not used as input or output
> +            2 - master, GPIO pin is used to drive a slave deserializer
> +            3 - slave, GPIO pin is used as FSYNC input driven by a master device
> +        enum: [0, 1, 2, 3]
> +        default: 0
> +      - description: |
> +          FSYNC TX ID: GPIO ID used for transmitting FSYNC signal
> +        minimum: 0
> +        maximum: 31
> +        default: 0
> +      - description: |
> +          FSYNC pin: 0 - MFP0, 1 - MFP7. Not used for internal mode.
> +        enum: [0, 1]
> +        default: 0
> +
>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
>  
> @@ -92,6 +134,9 @@ examples:
>      #include <dt-bindings/gpio/gpio.h>
>      #include <dt-bindings/media/video-interfaces.h>
>  
> +    maxim,operation-mode = <0>;
> +    maxim,fsync-config = <1 0>;
> +
>      i2c@e6508000 {
>              #address-cells = <1>;
>              #size-cells = <0>;
> -- 
> 2.44.1
> 

[RFC 03/12] staging: media: max96712: convert to using CCI register access helpers

[Laurentiu Palcu]

Use the CCI register access helpers instead of regmap's.

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
---
 drivers/staging/media/max96712/max96712.c | 126 +++++++++-------------
 1 file changed, 51 insertions(+), 75 deletions(-)

diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
index 47842facec125..cf39f5243cd6d 100644
--- a/drivers/staging/media/max96712/max96712.c
+++ b/drivers/staging/media/max96712/max96712.c
@@ -12,24 +12,25 @@
 #include <linux/of_graph.h>
 #include <linux/regmap.h>
 
+#include <media/v4l2-cci.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-fwnode.h>
 #include <media/v4l2-subdev.h>
 
 /* TOP_CTRL */
-#define MAX96712_DEBUG_EXTRA_REG			0x0009
+#define MAX96712_DEBUG_EXTRA_REG			CCI_REG8(0x0009)
 #define   DEBUG_EXTRA_PCLK_25MHZ			0x00
 #define   DEBUG_EXTRA_PCLK_75MHZ			0x01
-#define MAX96724_TOP_CTRL_PWR1				0x0013
+#define MAX96724_TOP_CTRL_PWR1				CCI_REG8(0x0013)
 #define   RESET_ALL					BIT(6)
 
 /* BACKTOP0 */
-#define MAX96712_BACKTOP0_12				0x040b
+#define MAX96712_BACKTOP0_12				CCI_REG8(0x040b)
 #define   CSI_OUT_EN					BIT(1)
 #define   SOFT_BPP_0_MASK				GENMASK(7, 3)
 #define   SOFT_BPP_0_SHIFT				3
-#define MAX96712_BACKTOP0_22				0x0415
-#define MAX96712_BACKTOP0_25				0x0418
+#define MAX96712_BACKTOP0_22				CCI_REG8(0x0415)
+#define MAX96712_BACKTOP0_25				CCI_REG8(0x0418)
 #define   PHY_CSI_TX_DPLL_PREDEF_FREQ_MASK		GENMASK(4, 0)
 #define   PHY_CSI_TX_DPLL_PREDEF_FREQ_SHIFT		0
 #define   PHY_CSI_TX_DPLL_FB_FRACTION_PREDEF_EN		BIT(5)
@@ -37,7 +38,7 @@
 #define   OVERRIDE_BPP_VC_DT_1_3			BIT(7)
 
 /* MIPI_PHY */
-#define MAX96712_MIPI_PHY_0				0x08a0
+#define MAX96712_MIPI_PHY_0				CCI_REG8(0x08a0)
 #define   PHY_4X2					BIT(0)
 #define   PHY_2X4					BIT(2)
 #define   PHY_1X4A_22					BIT(3)
@@ -45,7 +46,7 @@
 #define   FORCE_CLK0_EN					BIT(5)
 #define   FORCE_CLK3_EN					BIT(6)
 #define   FORCE_CSI_OUT_EN				BIT(7)
-#define MAX96712_MIPI_PHY_2				0x08a2
+#define MAX96712_MIPI_PHY_2				CCI_REG8(0x08a2)
 #define   T_HS_TRAIL_MASK				GENMASK(1, 0)
 #define   T_HS_TRAIL_SHIFT				0
 #define   T_LPX_MASK					GENMASK(3, 2)
@@ -56,22 +57,22 @@
 #define   PHY1_EN					BIT(5)
 #define   PHY2_EN					BIT(6)
 #define   PHY3_EN					BIT(7)
-#define MAX96712_MIPI_PHY_3				0x08a3
+#define MAX96712_MIPI_PHY_3				CCI_REG8(0x08a3)
 #define   PHY0_LANE_MAP_MASK				GENMASK(3, 0)
 #define   PHY0_LANE_MAP_SHIFT				0
 #define   PHY1_LANE_MAP_MASK				GENMASK(7, 4)
 #define   PHY1_LANE_MAP_SHIFT				4
-#define MAX96712_MIPI_PHY_5				0x08a5
+#define MAX96712_MIPI_PHY_5				CCI_REG8(0x08a5)
 #define   PHY0_POL_MAP_MASK				GENMASK(2, 0)
 #define   PHY0_POL_MAP_SHIFT				0
 #define   PHY1_POL_MAP_MASK				GENMASK(5, 3)
 #define   PHY1_POL_MAP_SHIFT				3
 #define   T_CLK_PREP_MASK				GENMASK(7, 6)
 #define   T_CLK_PREP_SHIFT				6
-#define MAX96712_MIPI_PHY_13				0x08ad
+#define MAX96712_MIPI_PHY_13				CCI_REG8(0x08ad)
 #define   T_T3_PREBEGIN_MASK				GENMASK(5, 0)
 #define   T_T3_PREBEGIN_SHIFT				0
-#define MAX96712_MIPI_PHY_14				0x08ae
+#define MAX96712_MIPI_PHY_14				CCI_REG8(0x08ae)
 #define   T_T3_PREP_MASK				GENMASK(1, 0)
 #define   T_T3_PREP_SHIFT				0
 #define   T_T3_PREP_40NS				0
@@ -82,7 +83,7 @@
 #define   T_T3_POST_SHIFT				2
 
 /* MIPI_TX: 0 <= phy < 4 */
-#define MAX96712_MIPI_TX_10(phy)			(0x090a + (phy) * 0x40)
+#define MAX96712_MIPI_TX_10(phy)			CCI_REG8(0x090a + (phy) * 0x40)
 #define   CSI2_TWAKEUP_H_MASK				GENMASK(2, 0)
 #define   CSI2_TWAKEUP_H_SHIFT				0
 #define   CSI2_VCX_EN					BIT(4)
@@ -91,7 +92,7 @@
 #define   CSI2_LANE_CNT_SHIFT				6
 
 /* VRX_PATGEN */
-#define MAX96712_VRX_PATGEN_0				0x1050
+#define MAX96712_VRX_PATGEN_0				CCI_REG8(0x1050)
 #define   VTG_MODE_MASK					GENMASK(1, 0)
 #define   VTG_MODE_SHIFT				0
 #define   VTG_MODE_VS_TRACKING				0
@@ -104,30 +105,30 @@
 #define   GEN_DE					BIT(5)
 #define   GEN_HS					BIT(6)
 #define   GEN_VS					BIT(7)
-#define MAX96712_VRX_PATGEN_1				0x1051
+#define MAX96712_VRX_PATGEN_1				CCI_REG8(0x1051)
 #define   VS_TRIG					BIT(0)
 #define   PATGEN_MODE_MASK				GENMASK(5, 4)
 #define   PATGEN_MODE_SHIFT				4
 #define   PATGEN_MODE_CHECKERBOARD			(1 << PATGEN_MODE_SHIFT)
 #define   PATGEN_MODE_GRADIENT				(2 << PATGEN_MODE_SHIFT)
 #define   GRAD_MODE					BIT(7)
-#define MAX96712_VRX_PATGEN_VS_DLY			0x1052
-#define MAX96712_VRX_PATGEN_VS_HIGH			0x1055
-#define MAX96712_VRX_PATGEN_VS_LOW			0x1058
-#define MAX96712_VRX_PATGEN_V2H				0x105b
-#define MAX96712_VRX_PATGEN_HS_HIGH			0x105e
-#define MAX96712_VRX_PATGEN_HS_LOW			0x1060
-#define MAX96712_VRX_PATGEN_HS_CNT			0x1062
-#define MAX96712_VRX_PATGEN_V2D				0x1064
-#define MAX96712_VRX_PATGEN_DE_HIGH			0x1067
-#define MAX96712_VRX_PATGEN_DE_LOW			0x1069
-#define MAX96712_VRX_PATGEN_DE_CNT			0x106b
-#define MAX96712_VRX_PATGEN_GRAD_INCR			0x106d
-#define MAX96712_VRX_PATGEN_CHKR_COLOR_A		0x106e
-#define MAX96712_VRX_PATGEN_CHKR_COLOR_B		0x1071
-#define MAX96712_VRX_PATGEN_CHKR_RPT_A			0x1074
-#define MAX96712_VRX_PATGEN_CHKR_RPT_B			0x1075
-#define MAX96712_VRX_PATGEN_CHKR_ALT			0x1076
+#define MAX96712_VRX_PATGEN_VS_DLY			CCI_REG24(0x1052)
+#define MAX96712_VRX_PATGEN_VS_HIGH			CCI_REG24(0x1055)
+#define MAX96712_VRX_PATGEN_VS_LOW			CCI_REG24(0x1058)
+#define MAX96712_VRX_PATGEN_V2H				CCI_REG16(0x105b)
+#define MAX96712_VRX_PATGEN_HS_HIGH			CCI_REG16(0x105e)
+#define MAX96712_VRX_PATGEN_HS_LOW			CCI_REG16(0x1060)
+#define MAX96712_VRX_PATGEN_HS_CNT			CCI_REG16(0x1062)
+#define MAX96712_VRX_PATGEN_V2D				CCI_REG24(0x1064)
+#define MAX96712_VRX_PATGEN_DE_HIGH			CCI_REG16(0x1067)
+#define MAX96712_VRX_PATGEN_DE_LOW			CCI_REG16(0x1069)
+#define MAX96712_VRX_PATGEN_DE_CNT			CCI_REG16(0x106b)
+#define MAX96712_VRX_PATGEN_GRAD_INCR			CCI_REG8(0x106d)
+#define MAX96712_VRX_PATGEN_CHKR_COLOR_A		CCI_REG24(0x106e)
+#define MAX96712_VRX_PATGEN_CHKR_COLOR_B		CCI_REG24(0x1071)
+#define MAX96712_VRX_PATGEN_CHKR_RPT_A			CCI_REG8(0x1074)
+#define MAX96712_VRX_PATGEN_CHKR_RPT_B			CCI_REG8(0x1075)
+#define MAX96712_VRX_PATGEN_CHKR_ALT			CCI_REG8(0x1076)
 
 enum max96712_pattern {
 	MAX96712_PATTERN_CHECKERBOARD = 0,
@@ -155,11 +156,11 @@ struct max96712_priv {
 	enum max96712_pattern pattern;
 };
 
-static int max96712_write(struct max96712_priv *priv, unsigned int reg, u8 val)
+static int max96712_write(struct max96712_priv *priv, unsigned int reg, u64 val)
 {
 	int ret;
 
-	ret = regmap_write(priv->regmap, reg, val);
+	ret = cci_write(priv->regmap, reg, val, NULL);
 	if (ret)
 		dev_err(&priv->client->dev, "write 0x%04x failed\n", reg);
 
@@ -167,42 +168,17 @@ static int max96712_write(struct max96712_priv *priv, unsigned int reg, u8 val)
 }
 
 static int max96712_update_bits(struct max96712_priv *priv, unsigned int reg,
-				u8 mask, u8 val)
+				u64 mask, u64 val)
 {
 	int ret;
 
-	ret = regmap_update_bits(priv->regmap, reg, mask, val);
+	ret = cci_update_bits(priv->regmap, reg, mask, val, NULL);
 	if (ret)
 		dev_err(&priv->client->dev, "update 0x%04x failed\n", reg);
 
 	return ret;
 }
 
-static int max96712_write_bulk(struct max96712_priv *priv, unsigned int reg,
-			       const void *val, size_t val_count)
-{
-	int ret;
-
-	ret = regmap_bulk_write(priv->regmap, reg, val, val_count);
-	if (ret)
-		dev_err(&priv->client->dev, "bulk write 0x%04x failed\n", reg);
-
-	return ret;
-}
-
-static int max96712_write_bulk_value(struct max96712_priv *priv,
-				     unsigned int reg, unsigned int val,
-				     size_t val_count)
-{
-	unsigned int i;
-	u8 values[4];
-
-	for (i = 1; i <= val_count; i++)
-		values[i - 1] = (val >> ((val_count - i) * 8)) & 0xff;
-
-	return max96712_write_bulk(priv, reg, &values, val_count);
-}
-
 static void max96712_reset(struct max96712_priv *priv)
 {
 	max96712_update_bits(priv, MAX96724_TOP_CTRL_PWR1, RESET_ALL, RESET_ALL);
@@ -293,19 +269,19 @@ static void max96712_pattern_enable(struct max96712_priv *priv, bool enable)
 	max96712_write(priv, MAX96712_DEBUG_EXTRA_REG, DEBUG_EXTRA_PCLK_75MHZ);
 
 	/* Configure Video Timing Generator for 1920x1080 @ 30 fps. */
-	max96712_write_bulk_value(priv, MAX96712_VRX_PATGEN_VS_DLY, 0, 3);
-	max96712_write_bulk_value(priv, MAX96712_VRX_PATGEN_VS_HIGH, v_sw * h_tot, 3);
-	max96712_write_bulk_value(priv, MAX96712_VRX_PATGEN_VS_LOW,
-				  (v_active + v_fp + v_bp) * h_tot, 3);
-	max96712_write_bulk_value(priv, MAX96712_VRX_PATGEN_V2H, 0, 3);
-	max96712_write_bulk_value(priv, MAX96712_VRX_PATGEN_HS_HIGH, h_sw, 2);
-	max96712_write_bulk_value(priv, MAX96712_VRX_PATGEN_HS_LOW, h_active + h_fp + h_bp, 2);
-	max96712_write_bulk_value(priv, MAX96712_VRX_PATGEN_HS_CNT, v_tot, 2);
-	max96712_write_bulk_value(priv, MAX96712_VRX_PATGEN_V2D,
-				  h_tot * (v_sw + v_bp) + (h_sw + h_bp), 3);
-	max96712_write_bulk_value(priv, MAX96712_VRX_PATGEN_DE_HIGH, h_active, 2);
-	max96712_write_bulk_value(priv, MAX96712_VRX_PATGEN_DE_LOW, h_fp + h_sw + h_bp, 2);
-	max96712_write_bulk_value(priv, MAX96712_VRX_PATGEN_DE_CNT, v_active, 2);
+	max96712_write(priv, MAX96712_VRX_PATGEN_VS_DLY, 0);
+	max96712_write(priv, MAX96712_VRX_PATGEN_VS_HIGH, v_sw * h_tot);
+	max96712_write(priv, MAX96712_VRX_PATGEN_VS_LOW,
+		       (v_active + v_fp + v_bp) * h_tot);
+	max96712_write(priv, MAX96712_VRX_PATGEN_V2H, 0);
+	max96712_write(priv, MAX96712_VRX_PATGEN_HS_HIGH, h_sw);
+	max96712_write(priv, MAX96712_VRX_PATGEN_HS_LOW, h_active + h_fp + h_bp);
+	max96712_write(priv, MAX96712_VRX_PATGEN_HS_CNT, v_tot);
+	max96712_write(priv, MAX96712_VRX_PATGEN_V2D,
+		       h_tot * (v_sw + v_bp) + (h_sw + h_bp));
+	max96712_write(priv, MAX96712_VRX_PATGEN_DE_HIGH, h_active);
+	max96712_write(priv, MAX96712_VRX_PATGEN_DE_LOW, h_fp + h_sw + h_bp);
+	max96712_write(priv, MAX96712_VRX_PATGEN_DE_CNT, v_active);
 
 	/* Generate VS, HS and DE in free-running mode. */
 	max96712_write(priv, MAX96712_VRX_PATGEN_0,
@@ -320,8 +296,8 @@ static void max96712_pattern_enable(struct max96712_priv *priv, bool enable)
 		max96712_write(priv, MAX96712_VRX_PATGEN_CHKR_ALT, 0x3c);
 
 		/* Set checkerboard pattern colors. */
-		max96712_write_bulk_value(priv, MAX96712_VRX_PATGEN_CHKR_COLOR_A, 0xfecc00, 3);
-		max96712_write_bulk_value(priv, MAX96712_VRX_PATGEN_CHKR_COLOR_B, 0x006aa7, 3);
+		max96712_write(priv, MAX96712_VRX_PATGEN_CHKR_COLOR_A, 0xfecc00);
+		max96712_write(priv, MAX96712_VRX_PATGEN_CHKR_COLOR_B, 0x006aa7);
 
 		/* Generate checkerboard pattern. */
 		max96712_write(priv, MAX96712_VRX_PATGEN_1, PATGEN_MODE_CHECKERBOARD);
-- 
2.44.1

[RFC 04/12] staging: media: max96712: change DT parsing routine

[Laurentiu Palcu]

Currently, the driver supports only one source media pad. In order to be
able to use the driver with other serializers we need sink media pads as
well. This patch adds support for parsing the source endpoints and
create the corresponding sink pads associated with the endpoints in DT.

The driver functionality is not changed at this point: only sink and
source pads are created. However, since the first source pad index is 4,
the user needs to make sure to link the source pad correctly to the next
downstream node.

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
---
 drivers/staging/media/max96712/max96712.c | 122 +++++++++++++++++++---
 1 file changed, 106 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
index cf39f5243cd6d..9c255979932d6 100644
--- a/drivers/staging/media/max96712/max96712.c
+++ b/drivers/staging/media/max96712/max96712.c
@@ -130,6 +130,13 @@
 #define MAX96712_VRX_PATGEN_CHKR_RPT_B			CCI_REG8(0x1075)
 #define MAX96712_VRX_PATGEN_CHKR_ALT			CCI_REG8(0x1076)
 
+#define MAX96712_MAX_RX_PORTS				4
+#define MAX96712_MAX_TX_PORTS				2
+#define MAX96712_MAX_VPG_PORTS				1
+#define MAX96712_MAX_PORTS				(MAX96712_MAX_RX_PORTS + \
+							 MAX96712_MAX_TX_PORTS + \
+							 MAX96712_MAX_VPG_PORTS)
+
 enum max96712_pattern {
 	MAX96712_PATTERN_CHECKERBOARD = 0,
 	MAX96712_PATTERN_GRADIENT,
@@ -139,6 +146,11 @@ struct max96712_info {
 	unsigned int dpllfreq;
 };
 
+struct max96712_rx_port {
+	struct v4l2_subdev *sd;
+	struct fwnode_handle *fwnode;
+};
+
 struct max96712_priv {
 	struct i2c_client *client;
 	struct regmap *regmap;
@@ -151,7 +163,11 @@ struct max96712_priv {
 
 	struct v4l2_subdev sd;
 	struct v4l2_ctrl_handler ctrl_handler;
-	struct media_pad pads[1];
+	struct media_pad pads[MAX96712_MAX_PORTS];
+
+	struct max96712_rx_port rx_ports[MAX96712_MAX_RX_PORTS];
+	unsigned int rx_port_mask;
+	unsigned int n_rx_ports;
 
 	enum max96712_pattern pattern;
 };
@@ -343,9 +359,12 @@ static int max96712_init_state(struct v4l2_subdev *sd,
 		.xfer_func      = V4L2_XFER_FUNC_DEFAULT,
 	};
 	struct v4l2_mbus_framefmt *fmt;
+	int i;
 
-	fmt = v4l2_subdev_state_get_format(state, 0);
-	*fmt = default_fmt;
+	for (i = 0; i < MAX96712_MAX_PORTS; i++) {
+		fmt = v4l2_subdev_state_get_format(state, i);
+		*fmt = default_fmt;
+	}
 
 	return 0;
 }
@@ -392,6 +411,7 @@ static int max96712_v4l2_register(struct max96712_priv *priv)
 {
 	long pixel_rate;
 	int ret;
+	int i;
 
 	priv->sd.internal_ops = &max96712_internal_ops;
 	v4l2_i2c_subdev_init(&priv->sd, priv->client, &max96712_subdev_ops);
@@ -418,8 +438,14 @@ static int max96712_v4l2_register(struct max96712_priv *priv)
 	if (ret)
 		goto error;
 
-	priv->pads[0].flags = MEDIA_PAD_FL_SOURCE;
-	ret = media_entity_pads_init(&priv->sd.entity, 1, priv->pads);
+	for (i = 0; i < MAX96712_MAX_RX_PORTS + MAX96712_MAX_TX_PORTS; i++)
+		priv->pads[i].flags = i < MAX96712_MAX_RX_PORTS ?
+				      MEDIA_PAD_FL_SINK : MEDIA_PAD_FL_SOURCE;
+
+	/* The last pad is the VPG pad. */
+	priv->pads[i].flags = MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_INTERNAL;
+
+	ret = media_entity_pads_init(&priv->sd.entity, MAX96712_MAX_PORTS, priv->pads);
 	if (ret)
 		goto error;
 
@@ -443,24 +469,53 @@ static int max96712_v4l2_register(struct max96712_priv *priv)
 	return ret;
 }
 
-static int max96712_parse_dt(struct max96712_priv *priv)
+static int max96712_parse_rx_ports(struct max96712_priv *priv, struct device_node *node,
+				   struct of_endpoint *ep)
+{
+	struct device *dev = &priv->client->dev;
+	struct max96712_rx_port *source;
+	struct fwnode_handle *remote_port_parent;
+
+	if (priv->rx_ports[ep->port].fwnode) {
+		dev_info(dev, "Multiple port endpoints are not supported: %d", ep->port);
+		return 0;
+	}
+
+	source = &priv->rx_ports[ep->port];
+	source->fwnode = fwnode_graph_get_remote_endpoint(of_fwnode_handle(node));
+	if (!source->fwnode) {
+		dev_info(dev, "Endpoint %pOF has no remote endpoint connection\n", ep->local_node);
+		return 0;
+	}
+
+	remote_port_parent = fwnode_graph_get_remote_port_parent(of_fwnode_handle(node));
+
+	if (!fwnode_device_is_available(remote_port_parent)) {
+		dev_dbg(dev, "Skipping port %d as remote port parent is disabled.\n",
+			ep->port);
+		source->fwnode = NULL;
+		goto fwnode_put;
+	}
+
+	priv->rx_port_mask |= BIT(ep->port);
+	priv->n_rx_ports++;
+
+fwnode_put:
+	fwnode_handle_put(remote_port_parent);
+	fwnode_handle_put(source->fwnode);
+	return 0;
+}
+
+static int max96712_parse_tx_ports(struct max96712_priv *priv, struct device_node *node,
+				   struct of_endpoint *ep)
 {
-	struct fwnode_handle *ep;
 	struct v4l2_fwnode_endpoint v4l2_ep = {
 		.bus_type = V4L2_MBUS_UNKNOWN,
 	};
 	unsigned int supported_lanes;
 	int ret;
 
-	ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(&priv->client->dev), 4,
-					     0, 0);
-	if (!ep) {
-		dev_err(&priv->client->dev, "Not connected to subdevice\n");
-		return -EINVAL;
-	}
-
-	ret = v4l2_fwnode_endpoint_parse(ep, &v4l2_ep);
-	fwnode_handle_put(ep);
+	ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(node), &v4l2_ep);
 	if (ret) {
 		dev_err(&priv->client->dev, "Could not parse v4l2 endpoint\n");
 		return -EINVAL;
@@ -492,6 +547,41 @@ static int max96712_parse_dt(struct max96712_priv *priv)
 	return 0;
 }
 
+static int max96712_parse_dt(struct max96712_priv *priv)
+{
+	struct device *dev = &priv->client->dev;
+	struct device_node *node = NULL;
+	int ret = 0;
+
+	for_each_endpoint_of_node(dev->of_node, node) {
+		struct of_endpoint ep;
+
+		of_graph_parse_endpoint(node, &ep);
+
+		if (ep.port >= MAX96712_MAX_PORTS) {
+			dev_err(dev, "Invalid endpoint %s on port %d",
+				of_node_full_name(ep.local_node), ep.port);
+			continue;
+		}
+
+		if (ep.port >= MAX96712_MAX_RX_PORTS) {
+			ret = max96712_parse_tx_ports(priv, node, &ep);
+			if (ret)
+				goto exit;
+
+			continue;
+		}
+
+		ret = max96712_parse_rx_ports(priv, node, &ep);
+		if (ret)
+			goto exit;
+	}
+
+exit:
+	of_node_put(node);
+	return ret;
+}
+
 static const struct regmap_config max96712_i2c_regmap = {
 	.reg_bits = 16,
 	.val_bits = 8,
-- 
2.44.1

Re: [RFC 04/12] staging: media: max96712: change DT parsing routine

[Dan Carpenter]

On Fri, Jan 31, 2025 at 06:33:58PM +0200, Laurentiu Palcu wrote:
> -static int max96712_parse_dt(struct max96712_priv *priv)
> +static int max96712_parse_rx_ports(struct max96712_priv *priv, struct device_node *node,
> +				   struct of_endpoint *ep)
> +{
> +	struct device *dev = &priv->client->dev;
> +	struct max96712_rx_port *source;
> +	struct fwnode_handle *remote_port_parent;
> +
> +	if (priv->rx_ports[ep->port].fwnode) {
> +		dev_info(dev, "Multiple port endpoints are not supported: %d", ep->port);
> +		return 0;
> +	}
> +
> +	source = &priv->rx_ports[ep->port];
> +	source->fwnode = fwnode_graph_get_remote_endpoint(of_fwnode_handle(node));
> +	if (!source->fwnode) {
> +		dev_info(dev, "Endpoint %pOF has no remote endpoint connection\n", ep->local_node);
> +		return 0;
> +	}
> +
> +	remote_port_parent = fwnode_graph_get_remote_port_parent(of_fwnode_handle(node));
> +
> +	if (!fwnode_device_is_available(remote_port_parent)) {
> +		dev_dbg(dev, "Skipping port %d as remote port parent is disabled.\n",
> +			ep->port);
> +		source->fwnode = NULL;

I don't understand the refcounting in this function.  Should we call
fwnode_handle_put(source->fwnode); before setting this to NULL?

> +		goto fwnode_put;
> +	}
> +
> +	priv->rx_port_mask |= BIT(ep->port);
> +	priv->n_rx_ports++;
> +
> +fwnode_put:
> +	fwnode_handle_put(remote_port_parent);
> +	fwnode_handle_put(source->fwnode);
> +	return 0;

I don't understand why we save source->fwnode but drop the refcount on
the success path.  My instinct says that it should be a source_fwnode
local stack variable instead.  (But again, I've only glanced at this
code so I could be wrong).


> +}
> +

regards,
dan carpenter

[RFC 05/12] staging: media: max96712: add link frequency V4L2 control

[Laurentiu Palcu]

The downstream nodes can use the V4L2_CID_PIXEL_RATE control to estimate
the link frequency but this can result in innacurate rates. Instead,
implement the V4L2_CID_LINK_FREQ control and pass the link frequency
from DT. If link-frequency DT property is missing fallback to using the
platform info DPLL value to compute the link frequency. Also, remove the
pixel rate control since it's not needed anymore.

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
---
 drivers/staging/media/max96712/max96712.c | 79 +++++++++++++++++------
 1 file changed, 60 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
index 9c255979932d6..546660e4b3d1e 100644
--- a/drivers/staging/media/max96712/max96712.c
+++ b/drivers/staging/media/max96712/max96712.c
@@ -83,7 +83,11 @@
 #define   T_T3_POST_SHIFT				2
 
 /* MIPI_TX: 0 <= phy < 4 */
-#define MAX96712_MIPI_TX_10(phy)			CCI_REG8(0x090a + (phy) * 0x40)
+#define MAX96712_MIPI_TX_DESKEW_INIT(phy)		CCI_REG8(0x0903 + (phy) * 0x40)
+#define   DPHY_DESKEW_AUTO_INIT_EN			BIT(7)
+#define MAX96712_MIPI_TX_DESKEW_PER(phy)		CCI_REG8(0x0904 + (phy) * 0x40)
+#define   PERIODIC_DESKEW_CALIBRATION_EN		BIT(7)
+#define MAX96712_MIPI_TX_10(phy)			(0x090a + (phy) * 0x40)
 #define   CSI2_TWAKEUP_H_MASK				GENMASK(2, 0)
 #define   CSI2_TWAKEUP_H_SHIFT				0
 #define   CSI2_VCX_EN					BIT(4)
@@ -137,6 +141,8 @@
 							 MAX96712_MAX_TX_PORTS + \
 							 MAX96712_MAX_VPG_PORTS)
 
+#define MHZ(f)						((f) * 1000000U)
+
 enum max96712_pattern {
 	MAX96712_PATTERN_CHECKERBOARD = 0,
 	MAX96712_PATTERN_GRADIENT,
@@ -160,6 +166,7 @@ struct max96712_priv {
 
 	bool cphy;
 	struct v4l2_mbus_config_mipi_csi2 mipi;
+	s64 link_freq;
 
 	struct v4l2_subdev sd;
 	struct v4l2_ctrl_handler ctrl_handler;
@@ -252,12 +259,28 @@ static void max96712_mipi_configure(struct max96712_priv *priv)
 			     PHY_CSI_TX_DPLL_FB_FRACTION_PREDEF_EN |
 			     PHY_CSI_TX_DPLL_PREDEF_FREQ_MASK,
 			     PHY_CSI_TX_DPLL_FB_FRACTION_PREDEF_EN |
-			     ((priv->info->dpllfreq / 100) & 0x1f));
+			     (((priv->link_freq * 2) / MHZ(100)) & 0x1f));
 	max96712_update_bits(priv, MAX96712_BACKTOP0_25,
 			     PHY_CSI_TX_DPLL_FB_FRACTION_PREDEF_EN |
 			     PHY_CSI_TX_DPLL_PREDEF_FREQ_MASK,
 			     PHY_CSI_TX_DPLL_FB_FRACTION_PREDEF_EN |
-			     ((priv->info->dpllfreq / 100) & 0x1f));
+			     (((priv->link_freq * 2) / MHZ(100)) & 0x1f));
+
+	/* disable deskew on PHY0 and PHY1 if D-PHY is used and DPLL <= 1500MHz */
+	if (!priv->cphy) {
+		u32 dpll = priv->link_freq * 2;
+		u8 auto_deskew_en = dpll > MHZ(1500) ? DPHY_DESKEW_AUTO_INIT_EN : 0;
+		u8 auto_deskew_calib_en = dpll > MHZ(1500) ? PERIODIC_DESKEW_CALIBRATION_EN : 0;
+
+		max96712_update_bits(priv, MAX96712_MIPI_TX_DESKEW_INIT(0),
+				     DPHY_DESKEW_AUTO_INIT_EN, auto_deskew_en);
+		max96712_update_bits(priv, MAX96712_MIPI_TX_DESKEW_PER(0),
+				     PERIODIC_DESKEW_CALIBRATION_EN, auto_deskew_calib_en);
+		max96712_update_bits(priv, MAX96712_MIPI_TX_DESKEW_INIT(1),
+				     DPHY_DESKEW_AUTO_INIT_EN, auto_deskew_en);
+		max96712_update_bits(priv, MAX96712_MIPI_TX_DESKEW_PER(1),
+				     PERIODIC_DESKEW_CALIBRATION_EN, auto_deskew_calib_en);
+	}
 
 	/* Enable PHY0 and PHY1 */
 	max96712_update_bits(priv, MAX96712_MIPI_PHY_2, PHY_STDBY_N_MASK, PHY0_EN | PHY1_EN);
@@ -409,7 +432,7 @@ static const struct v4l2_ctrl_ops max96712_ctrl_ops = {
 
 static int max96712_v4l2_register(struct max96712_priv *priv)
 {
-	long pixel_rate;
+	struct v4l2_ctrl *link_freq_ctrl;
 	int ret;
 	int i;
 
@@ -420,18 +443,15 @@ static int max96712_v4l2_register(struct max96712_priv *priv)
 
 	v4l2_ctrl_handler_init(&priv->ctrl_handler, 2);
 
-	/*
-	 * TODO: Once V4L2_CID_LINK_FREQ is changed from a menu control to an
-	 * INT64 control it should be used here instead of V4L2_CID_PIXEL_RATE.
-	 */
-	pixel_rate = priv->info->dpllfreq / priv->mipi.num_data_lanes * 1000000;
-	v4l2_ctrl_new_std(&priv->ctrl_handler, NULL, V4L2_CID_PIXEL_RATE,
-			  pixel_rate, pixel_rate, 1, pixel_rate);
+	v4l2_ctrl_new_int_menu(&priv->ctrl_handler, NULL, V4L2_CID_LINK_FREQ,
+			       0, 0, &priv->link_freq);
 
-	v4l2_ctrl_new_std_menu_items(&priv->ctrl_handler, &max96712_ctrl_ops,
-				     V4L2_CID_TEST_PATTERN,
-				     ARRAY_SIZE(max96712_test_pattern) - 1,
-				     0, 0, max96712_test_pattern);
+	link_freq_ctrl = v4l2_ctrl_new_std_menu_items(&priv->ctrl_handler, &max96712_ctrl_ops,
+						      V4L2_CID_TEST_PATTERN,
+						      ARRAY_SIZE(max96712_test_pattern) - 1,
+						      0, 0, max96712_test_pattern);
+	if (link_freq_ctrl)
+		link_freq_ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;
 
 	priv->sd.ctrl_handler = &priv->ctrl_handler;
 	ret = priv->ctrl_handler.error;
@@ -515,7 +535,7 @@ static int max96712_parse_tx_ports(struct max96712_priv *priv, struct device_nod
 	unsigned int supported_lanes;
 	int ret;
 
-	ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(node), &v4l2_ep);
+	ret = v4l2_fwnode_endpoint_alloc_parse(of_fwnode_handle(node), &v4l2_ep);
 	if (ret) {
 		dev_err(&priv->client->dev, "Could not parse v4l2 endpoint\n");
 		return -EINVAL;
@@ -533,18 +553,39 @@ static int max96712_parse_tx_ports(struct max96712_priv *priv, struct device_nod
 	default:
 		dev_err(&priv->client->dev, "Unsupported bus-type %u\n",
 			v4l2_ep.bus_type);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto free_v4l2_ep;
 	}
 
 	if (v4l2_ep.bus.mipi_csi2.num_data_lanes != supported_lanes) {
 		dev_err(&priv->client->dev, "Only %u data lanes supported\n",
 			supported_lanes);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto free_v4l2_ep;
+	}
+
+	if (v4l2_ep.nr_of_link_frequencies != 1) {
+		dev_info(&priv->client->dev,
+			 "No link frequencies provided in DT, use platform info.\n");
+		priv->link_freq = MHZ(priv->info->dpllfreq) / 2;
+	} else {
+		priv->link_freq = v4l2_ep.link_frequencies[0];
+
+		if (priv->link_freq < MHZ(100) || priv->link_freq > MHZ(1250) ||
+		    priv->link_freq % MHZ(50)) {
+			dev_err(&priv->client->dev,
+				"Link frequency must be a multiple of 50MHz.\n");
+			ret = -EINVAL;
+			goto free_v4l2_ep;
+		}
 	}
 
 	priv->mipi = v4l2_ep.bus.mipi_csi2;
 
-	return 0;
+free_v4l2_ep:
+	v4l2_fwnode_endpoint_free(&v4l2_ep);
+
+	return ret;
 }
 
 static int max96712_parse_dt(struct max96712_priv *priv)
-- 
2.44.1

[RFC 06/12] staging: media: max96712: add I2C mux support

[Laurentiu Palcu]

The deserializer chip allows communicating with remote serializers over
an I2C control channel within the GMSL link. However, to avoid address
collisions, we need to enable only the I2C CC corresponding to a
certain GMSL link and disable the other ones. Hence, add support for
I2C multiplexer which will allow us to do just that.

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
---
 drivers/staging/media/max96712/max96712.c | 76 ++++++++++++++++++++++-
 1 file changed, 75 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
index 546660e4b3d1e..f68a1d241b846 100644
--- a/drivers/staging/media/max96712/max96712.c
+++ b/drivers/staging/media/max96712/max96712.c
@@ -8,6 +8,7 @@
 
 #include <linux/delay.h>
 #include <linux/i2c.h>
+#include <linux/i2c-mux.h>
 #include <linux/module.h>
 #include <linux/of_graph.h>
 #include <linux/regmap.h>
@@ -17,6 +18,17 @@
 #include <media/v4l2-fwnode.h>
 #include <media/v4l2-subdev.h>
 
+/* DEV */
+#define MAX96712_DEV_REG3				CCI_REG8(0x0003)
+#define   DIS_REM_CC_A_MASK				GENMASK(1, 0)
+#define   DIS_REM_CC_A_SHIFT				0
+#define   DIS_REM_CC_B_MASK				GENMASK(3, 2)
+#define   DIS_REM_CC_B_SHIFT				2
+#define   DIS_REM_CC_C_MASK				GENMASK(5, 4)
+#define   DIS_REM_CC_C_SHIFT				4
+#define   DIS_REM_CC_D_MASK				GENMASK(7, 6)
+#define   DIS_REM_CC_D_SHIFT				6
+
 /* TOP_CTRL */
 #define MAX96712_DEBUG_EXTRA_REG			CCI_REG8(0x0009)
 #define   DEBUG_EXTRA_PCLK_25MHZ			0x00
@@ -162,6 +174,9 @@ struct max96712_priv {
 	struct regmap *regmap;
 	struct gpio_desc *gpiod_pwdn;
 
+	struct i2c_mux_core *mux;
+	int mux_chan;
+
 	const struct max96712_info *info;
 
 	bool cphy;
@@ -489,6 +504,61 @@ static int max96712_v4l2_register(struct max96712_priv *priv)
 	return ret;
 }
 
+static int max96712_i2c_mux_select(struct i2c_mux_core *muxc, u32 chan)
+{
+	struct max96712_priv *priv = i2c_mux_priv(muxc);
+	u8 val = 0xff;
+
+	if (priv->mux_chan == chan)
+		return 0;
+
+	val &= ~(0x3 << (chan * 2));
+	val |= 0x2 << (chan * 2);
+	max96712_write(priv, MAX96712_DEV_REG3, val);
+
+	priv->mux_chan = chan;
+
+	return 0;
+}
+
+static int max96712_i2c_init(struct max96712_priv *priv)
+{
+	int link;
+	int ret;
+
+	if (!i2c_check_functionality(priv->client->adapter, I2C_FUNC_SMBUS_WRITE_BYTE_DATA))
+		return -ENODEV;
+
+	priv->mux_chan = -1;
+
+	priv->mux = i2c_mux_alloc(priv->client->adapter, &priv->client->dev,
+				  priv->n_rx_ports, 0, I2C_MUX_LOCKED,
+				  max96712_i2c_mux_select, NULL);
+	if (!priv->mux) {
+		dev_err(&priv->client->dev, "Could not alloc I2C multiplexer.\n");
+		return -ENOMEM;
+	}
+
+	priv->mux->priv = priv;
+
+	for (link = 0; link < MAX96712_MAX_RX_PORTS; link++) {
+		if (!(priv->rx_port_mask & BIT(link)))
+			continue;
+
+		ret = i2c_mux_add_adapter(priv->mux, 0, link);
+		if (ret < 0) {
+			dev_err(&priv->client->dev, "Could not add I2C mux adapter.\n");
+			goto error;
+		}
+	}
+
+	return 0;
+
+error:
+	i2c_mux_del_adapters(priv->mux);
+	return ret;
+}
+
 static int max96712_parse_rx_ports(struct max96712_priv *priv, struct device_node *node,
 				   struct of_endpoint *ep)
 {
@@ -665,7 +735,11 @@ static int max96712_probe(struct i2c_client *client)
 
 	max96712_mipi_configure(priv);
 
-	return max96712_v4l2_register(priv);
+	ret = max96712_v4l2_register(priv);
+	if (ret)
+		return ret;
+
+	return max96712_i2c_init(priv);
 }
 
 static void max96712_remove(struct i2c_client *client)
-- 
2.44.1

[RFC 07/12] staging: media: max96712: add support for streams

[Laurentiu Palcu]

Since the chip supports 4 incoming GMSL links allowing for 4 sensors to
be connected, we need to add support for streams if we are to use more
than one sensor with this deserializer.

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
---
 drivers/staging/media/max96712/max96712.c | 177 +++++++++++++++++++---
 1 file changed, 158 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
index f68a1d241b846..a078e4c67c360 100644
--- a/drivers/staging/media/max96712/max96712.c
+++ b/drivers/staging/media/max96712/max96712.c
@@ -13,6 +13,7 @@
 #include <linux/of_graph.h>
 #include <linux/regmap.h>
 
+#include <media/mipi-csi2.h>
 #include <media/v4l2-cci.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-fwnode.h>
@@ -153,6 +154,10 @@
 							 MAX96712_MAX_TX_PORTS + \
 							 MAX96712_MAX_VPG_PORTS)
 
+#define MAX96712_FIRST_SOURCE_PAD			MAX96712_MAX_RX_PORTS
+#define MAX96712_VPG_PAD				(MAX96712_FIRST_SOURCE_PAD + \
+							 MAX96712_MAX_TX_PORTS)
+
 #define MHZ(f)						((f) * 1000000U)
 
 enum max96712_pattern {
@@ -194,6 +199,16 @@ struct max96712_priv {
 	enum max96712_pattern pattern;
 };
 
+static inline bool max96712_pad_is_sink(u32 pad)
+{
+	return pad < MAX96712_FIRST_SOURCE_PAD || pad == MAX96712_VPG_PAD;
+}
+
+static inline bool max96712_pad_is_source(u32 pad)
+{
+	return pad >= MAX96712_FIRST_SOURCE_PAD && pad < MAX96712_VPG_PAD;
+}
+
 static int max96712_write(struct max96712_priv *priv, unsigned int reg, u64 val)
 {
 	int ret;
@@ -364,27 +379,119 @@ static void max96712_pattern_enable(struct max96712_priv *priv, bool enable)
 	}
 }
 
-static int max96712_s_stream(struct v4l2_subdev *sd, int enable)
+static int max96712_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
+				   struct v4l2_mbus_frame_desc *fd)
 {
-	struct max96712_priv *priv = v4l2_get_subdevdata(sd);
+	struct max96712_priv *priv = container_of(sd, struct max96712_priv, sd);
+	struct device *dev = &priv->client->dev;
+	struct v4l2_subdev_state *state;
+	struct v4l2_subdev_route *route;
+	struct media_pad *remote_pad;
+	int ret = 0;
+	int i;
 
-	if (enable) {
-		max96712_pattern_enable(priv, true);
-		max96712_mipi_enable(priv, true);
-	} else {
-		max96712_mipi_enable(priv, false);
-		max96712_pattern_enable(priv, false);
+	if (!max96712_pad_is_source(pad))
+		return -EINVAL;
+
+	memset(fd, 0, sizeof(*fd));
+
+	fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
+
+	state = v4l2_subdev_lock_and_get_active_state(sd);
+
+	for_each_active_route(&state->routing, route) {
+		struct v4l2_mbus_frame_desc_entry *source_entry = NULL;
+		struct v4l2_mbus_frame_desc source_fd = {0};
+
+		if (route->source_pad != pad)
+			continue;
+
+		if (route->sink_pad == MAX96712_VPG_PAD) {
+			fd->entry[fd->num_entries].stream = route->source_stream;
+			fd->entry[fd->num_entries].pixelcode = MEDIA_BUS_FMT_RGB888_1X24;
+			fd->entry[fd->num_entries].bus.csi2.vc = 0;
+			fd->entry[fd->num_entries].bus.csi2.dt = MIPI_CSI2_DT_RGB888;
+			fd->num_entries++;
+			continue;
+		}
+
+		remote_pad = media_pad_remote_pad_first(&priv->pads[route->sink_pad]);
+		if (!remote_pad) {
+			dev_dbg(dev, "no remote pad found for sink pad\n");
+			ret = -EPIPE;
+			goto unlock_state;
+		}
+
+		ret = v4l2_subdev_call(priv->rx_ports[route->sink_pad].sd, pad, get_frame_desc,
+				       remote_pad->index, &source_fd);
+		if (ret) {
+			dev_err(dev, "Failed to get source frame desc for pad %u\n",
+				route->sink_pad);
+
+			goto unlock_state;
+		}
+
+		for (i = 0; i < source_fd.num_entries; i++) {
+			if (source_fd.entry[i].stream == route->sink_stream) {
+				source_entry = &source_fd.entry[i];
+				break;
+			}
+		}
+
+		if (!source_entry) {
+			dev_err(dev, "Failed to find stream from source frame desc\n");
+
+			ret = -EPIPE;
+			goto unlock_state;
+		}
+
+		fd->entry[fd->num_entries].stream = route->source_stream;
+		fd->entry[fd->num_entries].flags = source_entry->flags;
+		fd->entry[fd->num_entries].length = source_entry->length;
+		fd->entry[fd->num_entries].pixelcode = source_entry->pixelcode;
+		fd->entry[fd->num_entries].bus.csi2.vc = source_entry->bus.csi2.vc;
+		fd->entry[fd->num_entries].bus.csi2.dt = source_entry->bus.csi2.dt;
+
+		fd->num_entries++;
 	}
 
+unlock_state:
+	v4l2_subdev_unlock_state(state);
+
+	return ret;
+}
+
+static int max96712_enable_streams(struct v4l2_subdev *sd,
+				   struct v4l2_subdev_state *state,
+				   u32 source_pad, u64 streams_mask)
+{
+	struct max96712_priv *priv = v4l2_get_subdevdata(sd);
+
+	max96712_pattern_enable(priv, true);
+	max96712_mipi_enable(priv, true);
+
+	return 0;
+}
+
+static int max96712_disable_streams(struct v4l2_subdev *sd,
+				    struct v4l2_subdev_state *state,
+				    u32 source_pad, u64 streams_mask)
+{
+	struct max96712_priv *priv = v4l2_get_subdevdata(sd);
+
+	max96712_mipi_enable(priv, false);
+	max96712_pattern_enable(priv, false);
+
 	return 0;
 }
 
 static const struct v4l2_subdev_video_ops max96712_video_ops = {
-	.s_stream = max96712_s_stream,
+	.s_stream = v4l2_subdev_s_stream_helper,
 };
 
-static int max96712_init_state(struct v4l2_subdev *sd,
-			       struct v4l2_subdev_state *state)
+static int _max96712_set_routing(struct v4l2_subdev *sd,
+				 struct v4l2_subdev_state *state,
+				 struct v4l2_subdev_krouting *routing)
 {
 	static const struct v4l2_mbus_framefmt default_fmt = {
 		.width          = 1920,
@@ -396,15 +503,43 @@ static int max96712_init_state(struct v4l2_subdev *sd,
 		.quantization   = V4L2_QUANTIZATION_DEFAULT,
 		.xfer_func      = V4L2_XFER_FUNC_DEFAULT,
 	};
-	struct v4l2_mbus_framefmt *fmt;
-	int i;
+	int ret;
 
-	for (i = 0; i < MAX96712_MAX_PORTS; i++) {
-		fmt = v4l2_subdev_state_get_format(state, i);
-		*fmt = default_fmt;
-	}
+	ret = v4l2_subdev_routing_validate(sd, routing, V4L2_SUBDEV_ROUTING_ONLY_1_TO_1);
+	if (ret)
+		return ret;
 
-	return 0;
+	return v4l2_subdev_set_routing_with_fmt(sd, state, routing, &default_fmt);
+}
+
+static int max96712_set_routing(struct v4l2_subdev *sd, struct v4l2_subdev_state *state,
+				enum v4l2_subdev_format_whence which,
+				struct v4l2_subdev_krouting *routing)
+{
+	if (which == V4L2_SUBDEV_FORMAT_ACTIVE && media_entity_is_streaming(&sd->entity))
+		return -EBUSY;
+
+	return _max96712_set_routing(sd, state, routing);
+}
+
+static int max96712_init_state(struct v4l2_subdev *sd,
+			       struct v4l2_subdev_state *state)
+{
+	struct v4l2_subdev_route routes[] = {
+		{
+			.sink_pad = MAX96712_VPG_PAD,
+			.sink_stream = 0,
+			.source_pad = MAX96712_FIRST_SOURCE_PAD,
+			.source_stream = 0,
+			.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
+		},
+	};
+	struct v4l2_subdev_krouting routing = {
+		.num_routes = ARRAY_SIZE(routes),
+		.routes = routes,
+	};
+
+	return _max96712_set_routing(sd, state, &routing);
 }
 
 static const struct v4l2_subdev_internal_ops max96712_internal_ops = {
@@ -414,6 +549,10 @@ static const struct v4l2_subdev_internal_ops max96712_internal_ops = {
 static const struct v4l2_subdev_pad_ops max96712_pad_ops = {
 	.get_fmt = v4l2_subdev_get_fmt,
 	.set_fmt = v4l2_subdev_get_fmt,
+	.enable_streams = max96712_enable_streams,
+	.disable_streams = max96712_disable_streams,
+	.set_routing = max96712_set_routing,
+	.get_frame_desc = max96712_get_frame_desc,
 };
 
 static const struct v4l2_subdev_ops max96712_subdev_ops = {
@@ -453,7 +592,7 @@ static int max96712_v4l2_register(struct max96712_priv *priv)
 
 	priv->sd.internal_ops = &max96712_internal_ops;
 	v4l2_i2c_subdev_init(&priv->sd, priv->client, &max96712_subdev_ops);
-	priv->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+	priv->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_STREAMS;
 	priv->sd.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
 
 	v4l2_ctrl_handler_init(&priv->ctrl_handler, 2);
-- 
2.44.1

[RFC 08/12] staging: media: max96712: allow enumerating MBUS codes

[Laurentiu Palcu]

This would allow apps to enumerate the supported MBUS formats available
on a certain pad.

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
---
 drivers/staging/media/max96712/max96712.c | 142 ++++++++++++++++++++++
 1 file changed, 142 insertions(+)

diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
index a078e4c67c360..d735798effa5c 100644
--- a/drivers/staging/media/max96712/max96712.c
+++ b/drivers/staging/media/max96712/max96712.c
@@ -199,6 +199,107 @@ struct max96712_priv {
 	enum max96712_pattern pattern;
 };
 
+struct max96712_format_info {
+	u32 code;
+	u8 data_type;
+};
+
+static const struct max96712_format_info max96712_formats[] = {
+	/* YUV formats */
+	{
+		.code = MEDIA_BUS_FMT_UYVY8_1X16,
+		.data_type = MIPI_CSI2_DT_YUV422_8B,
+	}, {
+		.code = MEDIA_BUS_FMT_VYUY8_1X16,
+		.data_type = MIPI_CSI2_DT_YUV422_8B,
+	}, {
+		.code = MEDIA_BUS_FMT_YUYV8_1X16,
+		.data_type = MIPI_CSI2_DT_YUV422_8B,
+	}, {
+		.code = MEDIA_BUS_FMT_YVYU8_1X16,
+		.data_type = MIPI_CSI2_DT_YUV422_8B,
+	}, {
+		.code = MEDIA_BUS_FMT_UYVY10_1X20,
+		.data_type = MIPI_CSI2_DT_YUV422_10B,
+	}, {
+		.code = MEDIA_BUS_FMT_VYUY10_1X20,
+		.data_type = MIPI_CSI2_DT_YUV422_10B,
+	}, {
+		.code = MEDIA_BUS_FMT_YUYV10_1X20,
+		.data_type = MIPI_CSI2_DT_YUV422_10B,
+	}, {
+		.code = MEDIA_BUS_FMT_YVYU10_1X20,
+		.data_type = MIPI_CSI2_DT_YUV422_10B,
+	},
+	/* RGB formats */
+	{
+		.code = MEDIA_BUS_FMT_RGB565_1X16,
+		.data_type = MIPI_CSI2_DT_RGB565,
+	}, {
+		.code = MEDIA_BUS_FMT_BGR888_1X24,
+		.data_type = MIPI_CSI2_DT_RGB888,
+	}, {
+		.code = MEDIA_BUS_FMT_RGB888_1X24,
+		.data_type = MIPI_CSI2_DT_RGB888,
+	}, {
+		.code = MEDIA_BUS_FMT_RBG888_1X24,
+		.data_type = MIPI_CSI2_DT_RGB888,
+	}, {
+		.code = MEDIA_BUS_FMT_GBR888_1X24,
+		.data_type = MIPI_CSI2_DT_RGB888,
+	},
+	/* RAW formats */
+	{
+		.code = MEDIA_BUS_FMT_SBGGR8_1X8,
+		.data_type = MIPI_CSI2_DT_RAW8,
+	}, {
+		.code = MEDIA_BUS_FMT_SGBRG8_1X8,
+		.data_type = MIPI_CSI2_DT_RAW8,
+	}, {
+		.code = MEDIA_BUS_FMT_SGRBG8_1X8,
+		.data_type = MIPI_CSI2_DT_RAW8,
+	}, {
+		.code = MEDIA_BUS_FMT_SRGGB8_1X8,
+		.data_type = MIPI_CSI2_DT_RAW8,
+	}, {
+		.code = MEDIA_BUS_FMT_SBGGR12_1X12,
+		.data_type = MIPI_CSI2_DT_RAW12,
+	}, {
+		.code = MEDIA_BUS_FMT_SGBRG12_1X12,
+		.data_type = MIPI_CSI2_DT_RAW12,
+	}, {
+		.code = MEDIA_BUS_FMT_SGRBG12_1X12,
+		.data_type = MIPI_CSI2_DT_RAW12,
+	}, {
+		.code = MEDIA_BUS_FMT_SRGGB12_1X12,
+		.data_type = MIPI_CSI2_DT_RAW12,
+	}, {
+		.code = MEDIA_BUS_FMT_SBGGR14_1X14,
+		.data_type = MIPI_CSI2_DT_RAW14,
+	}, {
+		.code = MEDIA_BUS_FMT_SGBRG14_1X14,
+		.data_type = MIPI_CSI2_DT_RAW14,
+	}, {
+		.code = MEDIA_BUS_FMT_SGRBG14_1X14,
+		.data_type = MIPI_CSI2_DT_RAW14,
+	}, {
+		.code = MEDIA_BUS_FMT_SRGGB14_1X14,
+		.data_type = MIPI_CSI2_DT_RAW14,
+	}, {
+		.code = MEDIA_BUS_FMT_SBGGR16_1X16,
+		.data_type = MIPI_CSI2_DT_RAW16,
+	}, {
+		.code = MEDIA_BUS_FMT_SGBRG16_1X16,
+		.data_type = MIPI_CSI2_DT_RAW16,
+	}, {
+		.code = MEDIA_BUS_FMT_SGRBG16_1X16,
+		.data_type = MIPI_CSI2_DT_RAW16,
+	}, {
+		.code = MEDIA_BUS_FMT_SRGGB16_1X16,
+		.data_type = MIPI_CSI2_DT_RAW16,
+	},
+};
+
 static inline bool max96712_pad_is_sink(u32 pad)
 {
 	return pad < MAX96712_FIRST_SOURCE_PAD || pad == MAX96712_VPG_PAD;
@@ -379,6 +480,46 @@ static void max96712_pattern_enable(struct max96712_priv *priv, bool enable)
 	}
 }
 
+static int max96712_enum_mbus_code(struct v4l2_subdev *sd, struct v4l2_subdev_state *sd_state,
+				   struct v4l2_subdev_mbus_code_enum *code)
+{
+	if (code->pad > MAX96712_VPG_PAD)
+		return -EINVAL;
+
+	if (max96712_pad_is_source(code->pad)) {
+		struct v4l2_mbus_framefmt *fmt;
+
+		if (code->index > 0)
+			return -EINVAL;
+
+		fmt = v4l2_subdev_state_get_opposite_stream_format(sd_state, code->pad,
+								   code->stream);
+		if (!fmt)
+			return -EINVAL;
+
+		code->code = fmt->code;
+
+		return 0;
+	}
+
+	/* Internal VPG pad only supprts RGB888 */
+	if (code->pad == MAX96712_VPG_PAD) {
+		if (code->index > 0)
+			return -EINVAL;
+
+		code->code = MEDIA_BUS_FMT_RGB888_1X24;
+
+		return 0;
+	}
+
+	if (code->index >= ARRAY_SIZE(max96712_formats))
+		return -EINVAL;
+
+	code->code = max96712_formats[code->index].code;
+
+	return 0;
+}
+
 static int max96712_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
 				   struct v4l2_mbus_frame_desc *fd)
 {
@@ -547,6 +688,7 @@ static const struct v4l2_subdev_internal_ops max96712_internal_ops = {
 };
 
 static const struct v4l2_subdev_pad_ops max96712_pad_ops = {
+	.enum_mbus_code	= max96712_enum_mbus_code,
 	.get_fmt = v4l2_subdev_get_fmt,
 	.set_fmt = v4l2_subdev_get_fmt,
 	.enable_streams = max96712_enable_streams,
-- 
2.44.1

[RFC 09/12] staging: media: max96712: add set_fmt routine

[Laurentiu Palcu]

Allow apps to change the format of the pads. Also, use the provided
width and height when generating the test pattern.

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
---
 drivers/staging/media/max96712/max96712.c | 57 ++++++++++++++++++++---
 1 file changed, 51 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
index d735798effa5c..ed1d46ea98cb9 100644
--- a/drivers/staging/media/max96712/max96712.c
+++ b/drivers/staging/media/max96712/max96712.c
@@ -417,15 +417,18 @@ static void max96712_mipi_configure(struct max96712_priv *priv)
 	max96712_update_bits(priv, MAX96712_MIPI_PHY_2, PHY_STDBY_N_MASK, PHY0_EN | PHY1_EN);
 }
 
-static void max96712_pattern_enable(struct max96712_priv *priv, bool enable)
+static void max96712_pattern_enable(struct max96712_priv *priv, struct v4l2_subdev_state *state,
+				    bool enable)
 {
-	const u32 h_active = 1920;
+	struct v4l2_mbus_framefmt *fmt = v4l2_subdev_state_get_format(state, MAX96712_VPG_PAD);
+
+	const u32 h_active = fmt->width;
 	const u32 h_fp = 88;
 	const u32 h_sw = 44;
 	const u32 h_bp = 148;
 	const u32 h_tot = h_active + h_fp + h_sw + h_bp;
 
-	const u32 v_active = 1080;
+	const u32 v_active = fmt->height;
 	const u32 v_fp = 4;
 	const u32 v_sw = 5;
 	const u32 v_bp = 36;
@@ -608,7 +611,7 @@ static int max96712_enable_streams(struct v4l2_subdev *sd,
 {
 	struct max96712_priv *priv = v4l2_get_subdevdata(sd);
 
-	max96712_pattern_enable(priv, true);
+	max96712_pattern_enable(priv, state, true);
 	max96712_mipi_enable(priv, true);
 
 	return 0;
@@ -621,7 +624,7 @@ static int max96712_disable_streams(struct v4l2_subdev *sd,
 	struct max96712_priv *priv = v4l2_get_subdevdata(sd);
 
 	max96712_mipi_enable(priv, false);
-	max96712_pattern_enable(priv, false);
+	max96712_pattern_enable(priv, state, false);
 
 	return 0;
 }
@@ -663,6 +666,48 @@ static int max96712_set_routing(struct v4l2_subdev *sd, struct v4l2_subdev_state
 	return _max96712_set_routing(sd, state, routing);
 }
 
+static int max96712_set_fmt(struct v4l2_subdev *sd,
+			    struct v4l2_subdev_state *state,
+			    struct v4l2_subdev_format *format)
+{
+	struct v4l2_mbus_framefmt *fmt;
+	int i;
+
+	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE && media_entity_is_streaming(&sd->entity))
+		return -EBUSY;
+
+	/* No transcoding, source and sink formats must match. */
+	if (max96712_pad_is_source(format->pad))
+		return v4l2_subdev_get_fmt(sd, state, format);
+
+	/* Validate the format. */
+	for (i = 0; i < ARRAY_SIZE(max96712_formats); ++i) {
+		if (max96712_formats[i].code == format->format.code)
+			break;
+	}
+
+	if (i == ARRAY_SIZE(max96712_formats))
+		format->format.code = max96712_formats[12].code;
+
+	if (format->pad == MAX96712_VPG_PAD && format->format.code != MEDIA_BUS_FMT_RGB888_1X24)
+		return -EINVAL;
+
+	fmt = v4l2_subdev_state_get_format(state, format->pad, format->stream);
+	if (!fmt)
+		return -EINVAL;
+
+	*fmt = format->format;
+
+	fmt = v4l2_subdev_state_get_opposite_stream_format(state, format->pad,
+							   format->stream);
+	if (!fmt)
+		return -EINVAL;
+
+	*fmt = format->format;
+
+	return 0;
+}
+
 static int max96712_init_state(struct v4l2_subdev *sd,
 			       struct v4l2_subdev_state *state)
 {
@@ -690,7 +735,7 @@ static const struct v4l2_subdev_internal_ops max96712_internal_ops = {
 static const struct v4l2_subdev_pad_ops max96712_pad_ops = {
 	.enum_mbus_code	= max96712_enum_mbus_code,
 	.get_fmt = v4l2_subdev_get_fmt,
-	.set_fmt = v4l2_subdev_get_fmt,
+	.set_fmt = max96712_set_fmt,
 	.enable_streams = max96712_enable_streams,
 	.disable_streams = max96712_disable_streams,
 	.set_routing = max96712_set_routing,
-- 
2.44.1

[RFC 10/12] staging: media: max96712: add gpiochip functionality

[Laurentiu Palcu]

The deserializer has GPIOs that can be used for various purposes. Add
support for gpiochip.

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
---
 drivers/staging/media/max96712/max96712.c | 140 ++++++++++++++++++++++
 1 file changed, 140 insertions(+)

diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
index ed1d46ea98cb9..307b2f1d3a6be 100644
--- a/drivers/staging/media/max96712/max96712.c
+++ b/drivers/staging/media/max96712/max96712.c
@@ -7,6 +7,7 @@
  */
 
 #include <linux/delay.h>
+#include <linux/gpio/driver.h>
 #include <linux/i2c.h>
 #include <linux/i2c-mux.h>
 #include <linux/module.h>
@@ -108,6 +109,41 @@
 #define   CSI2_LANE_CNT_MASK				GENMASK(7, 6)
 #define   CSI2_LANE_CNT_SHIFT				6
 
+/* GPIO_A: 0 <= gpio < 11 */
+#define MAX96712_GPIO_A_A(gpio)				CCI_REG8(0x0300 + (gpio) * 0x03)
+#define   GPIO_OUT_DIS					BIT(0)
+#define   GPIO_TX_EN_A					BIT(1)
+#define   GPIO_RX_EN_A					BIT(2)
+#define   GPIO_IN					BIT(3)
+#define   GPIO_OUT					BIT(4)
+#define   TX_COMP_EN_A					BIT(5)
+#define   RES_CFG					BIT(7)
+#define MAX96712_GPIO_A_B(gpio)				CCI_REG8(0x0301 + (gpio) * 0x03)
+#define   GPIO_TX_ID_A_MASK				GENMASK(4, 0)
+#define   GPIO_TX_ID_A_SHIFT				0
+#define   OUT_TYPE					BIT(5)
+#define   PULL_UPDN_SEL_MASK				GENMASK(7, 6)
+#define   PULL_UPDN_SEL_SHIFT				6
+#define MAX96712_GPIO_A_C(gpio)				CCI_REG8(0x0302 + (gpio) * 0x03)
+#define   GPIO_RX_ID_A_MASK				GENMASK(4, 0)
+#define   GPIO_RX_ID_A_SHIFT				0
+#define   GPIO_RECVED_A					BIT(6)
+#define   OVR_RES_CFG					BIT(7)
+
+/* GPIO_B, GPIO_C, GPIO_D: 0 <= gpio < 11, link: 1, 2, 3 */
+#define MAX96712_GPIO_B(gpio)				CCI_REG8(0x0301 + (link) * 0x36 + \
+								 (gpio) * 0x03)
+#define   GPIO_TX_ID_MASK				GENMASK(4, 0)
+#define   GPIO_TX_ID_SHIFT				0
+#define   GPIO_TX_EN					BIT(5)
+#define   TX_COMP_EN					BIT(6)
+#define MAX96712_GPIO_C(gpio)				CCI_REG8(0x0302 + (link) * 0x36 + \
+								 (gpio) * 0x03)
+#define   GPIO_RX_ID_MASK				GENMASK(4, 0)
+#define   GPIO_RX_ID_SHIFT				0
+#define   GPIO_RX_EN					BIT(5)
+#define   GPIO_RECVED					BIT(6)
+
 /* VRX_PATGEN */
 #define MAX96712_VRX_PATGEN_0				CCI_REG8(0x1050)
 #define   VTG_MODE_MASK					GENMASK(1, 0)
@@ -160,6 +196,8 @@
 
 #define MHZ(f)						((f) * 1000000U)
 
+#define MAX96712_NUM_GPIO				12
+
 enum max96712_pattern {
 	MAX96712_PATTERN_CHECKERBOARD = 0,
 	MAX96712_PATTERN_GRADIENT,
@@ -179,6 +217,8 @@ struct max96712_priv {
 	struct regmap *regmap;
 	struct gpio_desc *gpiod_pwdn;
 
+	struct gpio_chip gpio_chip;
+
 	struct i2c_mux_core *mux;
 	int mux_chan;
 
@@ -830,6 +870,7 @@ static int max96712_v4l2_register(struct max96712_priv *priv)
 	return ret;
 }
 
+/* I2C Mux section */
 static int max96712_i2c_mux_select(struct i2c_mux_core *muxc, u32 chan)
 {
 	struct max96712_priv *priv = i2c_mux_priv(muxc);
@@ -885,6 +926,101 @@ static int max96712_i2c_init(struct max96712_priv *priv)
 	return ret;
 }
 
+/* GPIO chip section */
+static int max96712_gpiochip_get(struct gpio_chip *gpiochip,
+				 unsigned int offset)
+{
+	struct max96712_priv *priv = gpiochip_get_data(gpiochip);
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(priv->regmap, MAX96712_GPIO_A_A(offset), &val);
+	if (ret)
+		return ret;
+
+	if (val & GPIO_OUT_DIS)
+		return !!(val & GPIO_IN);
+	else
+		return !!(val & GPIO_OUT);
+}
+
+static void max96712_gpiochip_set(struct gpio_chip *gpiochip,
+				  unsigned int offset, int value)
+{
+	struct max96712_priv *priv = gpiochip_get_data(gpiochip);
+
+	regmap_update_bits(priv->regmap, MAX96712_GPIO_A_A(offset), GPIO_OUT,
+			   GPIO_OUT);
+}
+
+static int max96712_gpio_get_direction(struct gpio_chip *gpiochip,
+				       unsigned int offset)
+{
+	struct max96712_priv *priv = gpiochip_get_data(gpiochip);
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(priv->regmap, MAX96712_GPIO_A_A(offset), &val);
+	if (ret < 0)
+		return ret;
+
+	return !!(val & GPIO_OUT_DIS);
+}
+
+static int max96712_gpio_direction_out(struct gpio_chip *gpiochip,
+				       unsigned int offset, int value)
+{
+	struct max96712_priv *priv = gpiochip_get_data(gpiochip);
+
+	return regmap_update_bits(priv->regmap, MAX96712_GPIO_A_A(offset),
+				  GPIO_OUT_DIS | GPIO_OUT,
+				  value ? GPIO_OUT : 0);
+}
+
+static int max96712_gpio_direction_in(struct gpio_chip *gpiochip,
+				      unsigned int offset)
+{
+	struct max96712_priv *priv = gpiochip_get_data(gpiochip);
+
+	return regmap_update_bits(priv->regmap, MAX96712_GPIO_A_A(offset),
+				  GPIO_OUT_DIS, GPIO_OUT_DIS);
+}
+
+static int max96712_gpiochip_probe(struct max96712_priv *priv)
+{
+	struct device *dev = &priv->client->dev;
+	struct gpio_chip *gc = &priv->gpio_chip;
+	int i, ret = 0;
+
+	gc->label = dev_name(dev);
+	gc->parent = dev;
+	gc->owner = THIS_MODULE;
+	gc->ngpio = MAX96712_NUM_GPIO;
+	gc->base = -1;
+	gc->can_sleep = true;
+	gc->get_direction = max96712_gpio_get_direction;
+	gc->direction_input = max96712_gpio_direction_in;
+	gc->direction_output = max96712_gpio_direction_out;
+	gc->request = gpiochip_generic_request;
+	gc->set = max96712_gpiochip_set;
+	gc->get = max96712_gpiochip_get;
+	gc->of_gpio_n_cells = 2;
+
+	/* Disable GPIO forwarding */
+	for (i = 0; i < gc->ngpio; i++)
+		regmap_update_bits(priv->regmap, MAX96712_GPIO_A_A(i),
+				   GPIO_RX_EN_A | GPIO_TX_EN_A, 0);
+
+	ret = devm_gpiochip_add_data(dev, gc, priv);
+	if (ret) {
+		dev_err(dev, "Unable to create gpio_chip\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+/* DT parsing section */
 static int max96712_parse_rx_ports(struct max96712_priv *priv, struct device_node *node,
 				   struct of_endpoint *ep)
 {
@@ -1061,6 +1197,10 @@ static int max96712_probe(struct i2c_client *client)
 
 	max96712_mipi_configure(priv);
 
+	ret = max96712_gpiochip_probe(priv);
+	if (ret)
+		return ret;
+
 	ret = max96712_v4l2_register(priv);
 	if (ret)
 		return ret;
-- 
2.44.1

Re: [RFC 10/12] staging: media: max96712: add gpiochip functionality

[Linus Walleij]

Hi Laurentiu,

thanks for your patch!

On Fri, Jan 31, 2025 at 5:35 PM Laurentiu Palcu
<laurentiu.palcu@oss.nxp.com> wrote:

> The deserializer has GPIOs that can be used for various purposes. Add
> support for gpiochip.
>
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>

Since you are using CONFIG_GPIOLIB unconditionally you need
to add

select GPIOLIB

in the Kconfig for this driver, or the autobuilder will soon start to
spam you with compilation errors.

> +static int max96712_gpiochip_probe(struct max96712_priv *priv)
> +{
> +       struct device *dev = &priv->client->dev;
> +       struct gpio_chip *gc = &priv->gpio_chip;
> +       int i, ret = 0;
> +
> +       gc->label = dev_name(dev);
> +       gc->parent = dev;

I don't think you need to assign parent. (Default)

> +       gc->owner = THIS_MODULE;

Or this. (Default)

> +       gc->ngpio = MAX96712_NUM_GPIO;
> +       gc->base = -1;
> +       gc->can_sleep = true;
> +       gc->get_direction = max96712_gpio_get_direction;
> +       gc->direction_input = max96712_gpio_direction_in;
> +       gc->direction_output = max96712_gpio_direction_out;
> +       gc->request = gpiochip_generic_request;
> +       gc->set = max96712_gpiochip_set;
> +       gc->get = max96712_gpiochip_get;
> +       gc->of_gpio_n_cells = 2;

Isn't that the default? Do you need to assign this?

Other than that this looks good, so with the small fix above:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[RFC 11/12] staging: media: max96712: add fsync support

[Laurentiu Palcu]

FSYNC is used to align images sent from multiple sensors.

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
---
 drivers/staging/media/max96712/max96712.c | 119 +++++++++++++++++++++-
 1 file changed, 118 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
index 307b2f1d3a6be..0112052171b06 100644
--- a/drivers/staging/media/max96712/max96712.c
+++ b/drivers/staging/media/max96712/max96712.c
@@ -51,6 +51,21 @@
 #define   OVERRIDE_BPP_VC_DT_0_2			BIT(6)
 #define   OVERRIDE_BPP_VC_DT_1_3			BIT(7)
 
+/* FSYNC */
+#define MAX96712_FSYNC_0				CCI_REG8(0x04a0)
+#define   FSYNC_METH_MASK				GENMASK(1, 0)
+#define   FSYNC_METH_SHIFT				0
+#define   FSYNC_MODE_MASK				GENMASK(3, 2)
+#define   FSYNC_MODE_SHIFT				2
+#define   EN_VS_GEN					BIT(4)
+#define   FSYNC_OUT_PIN					BIT(5)
+#define MAX96712_FSYNC_PERIOD				CCI_REG24_LE(0x04a5)
+#define MAX96712_FSYNC_17				CCI_REG8(0x04b1)
+#define   FSYNC_ERR_THR_MASK				GENMASK(2, 0)
+#define   FSYNC_ERR_THR_SHIFT				0
+#define   FSYNC_TX_ID_MASK				GENMASK(7, 3)
+#define   FSYNC_TX_ID_SHIFT				3
+
 /* MIPI_PHY */
 #define MAX96712_MIPI_PHY_0				CCI_REG8(0x08a0)
 #define   PHY_4X2					BIT(0)
@@ -194,6 +209,7 @@
 #define MAX96712_VPG_PAD				(MAX96712_FIRST_SOURCE_PAD + \
 							 MAX96712_MAX_TX_PORTS)
 
+#define MAX96712_XTAL_CLOCK				25000000ULL
 #define MHZ(f)						((f) * 1000000U)
 
 #define MAX96712_NUM_GPIO				12
@@ -207,6 +223,19 @@ struct max96712_info {
 	unsigned int dpllfreq;
 };
 
+enum max96712_fsync_mode {
+	MAX96712_FSYNC_OFF = 0,
+	MAX96712_FSYNC_INTERNAL,
+	MAX96712_FSYNC_MASTER,
+	MAX96712_FSYNC_SLAVE,
+};
+
+struct max96712_fsync {
+	int pin;
+	enum max96712_fsync_mode mode;
+	int tx_id;
+};
+
 struct max96712_rx_port {
 	struct v4l2_subdev *sd;
 	struct fwnode_handle *fwnode;
@@ -237,6 +266,9 @@ struct max96712_priv {
 	unsigned int n_rx_ports;
 
 	enum max96712_pattern pattern;
+
+	struct max96712_fsync fsync;
+	struct v4l2_fract interval;
 };
 
 struct max96712_format_info {
@@ -523,6 +555,68 @@ static void max96712_pattern_enable(struct max96712_priv *priv, struct v4l2_subd
 	}
 }
 
+static int __maybe_unused max96712_fsync_set(struct max96712_priv *priv)
+{
+	u32 fsync;
+	int ret;
+	u8 mode_map[4] = {3, 0, 1, 2};
+
+	if (priv->fsync.mode == MAX96712_FSYNC_OFF)
+		return 0;
+
+	if (!priv->interval.numerator || !priv->interval.denominator)
+		return max96712_update_bits(priv, MAX96712_FSYNC_0,
+					    FSYNC_METH_MASK | FSYNC_MODE_MASK,
+					    0x3 << FSYNC_MODE_SHIFT);
+
+	/*
+	 * According to Max96724 users guide, "sync signal frequency must be specified in terms of
+	 * the onboard crystal clock (25MHz)"
+	 */
+	fsync = div_u64(MAX96712_XTAL_CLOCK * priv->interval.numerator, priv->interval.denominator);
+
+	ret = max96712_write(priv, MAX96712_FSYNC_PERIOD, fsync);
+
+	ret |= max96712_update_bits(priv, MAX96712_FSYNC_0,
+				    FSYNC_OUT_PIN | FSYNC_METH_MASK | FSYNC_MODE_MASK,
+				    (priv->fsync.pin ? FSYNC_OUT_PIN : 0) |
+				    (0x0 << FSYNC_METH_SHIFT) |
+				    (mode_map[priv->fsync.mode] << FSYNC_MODE_SHIFT));
+
+	ret |= max96712_update_bits(priv, MAX96712_FSYNC_17, FSYNC_TX_ID_MASK,
+				    priv->fsync.tx_id << FSYNC_TX_ID_SHIFT);
+
+	return ret ? -EIO : 0;
+}
+
+static int max96712_get_frame_interval(struct v4l2_subdev *sd,
+				       struct v4l2_subdev_state *state,
+				       struct v4l2_subdev_frame_interval *interval)
+{
+	struct max96712_priv *priv = container_of(sd, struct max96712_priv, sd);
+
+	if (!max96712_pad_is_source(interval->pad))
+		return -EINVAL;
+
+	interval->interval = priv->interval;
+
+	return 0;
+}
+
+static int max96712_set_frame_interval(struct v4l2_subdev *sd,
+				       struct v4l2_subdev_state *state,
+				       struct v4l2_subdev_frame_interval *interval)
+{
+	struct max96712_priv *priv = container_of(sd, struct max96712_priv, sd);
+
+	if (!max96712_pad_is_source(interval->pad))
+		return -EINVAL;
+
+	priv->interval = interval->interval;
+
+	return 0;
+}
+
 static int max96712_enum_mbus_code(struct v4l2_subdev *sd, struct v4l2_subdev_state *sd_state,
 				   struct v4l2_subdev_mbus_code_enum *code)
 {
@@ -780,6 +874,8 @@ static const struct v4l2_subdev_pad_ops max96712_pad_ops = {
 	.disable_streams = max96712_disable_streams,
 	.set_routing = max96712_set_routing,
 	.get_frame_desc = max96712_get_frame_desc,
+	.get_frame_interval	= max96712_get_frame_interval,
+	.set_frame_interval	= max96712_set_frame_interval,
 };
 
 static const struct v4l2_subdev_ops max96712_subdev_ops = {
@@ -1124,7 +1220,28 @@ static int max96712_parse_dt(struct max96712_priv *priv)
 {
 	struct device *dev = &priv->client->dev;
 	struct device_node *node = NULL;
-	int ret = 0;
+	int ret = 0, count;
+	u32 dt_val[3];
+
+	count = fwnode_property_count_u32(dev_fwnode(dev), "maxim,fsync-config");
+	if (count > 0) {
+		ret = fwnode_property_read_u32_array(dev_fwnode(dev), "maxim,fsync-config",
+						     dt_val, count);
+		if (ret) {
+			dev_err(dev, "Unable to read FSYNC config from DT.\n");
+			return ret;
+		}
+
+		priv->fsync.mode = dt_val[0];
+		priv->fsync.tx_id = priv->fsync.mode ? dt_val[1] : 0;
+
+		if (priv->fsync.mode >= MAX96712_FSYNC_MASTER && count == 2) {
+			dev_err(dev, "No FSYNC pin provided in DT for the given mode.\n");
+			return -EINVAL;
+		}
+
+		priv->fsync.pin = dt_val[2];
+	}
 
 	for_each_endpoint_of_node(dev->of_node, node) {
 		struct of_endpoint ep;
-- 
2.44.1

[RFC 12/12] staging: media: max96712: allow streaming from connected sensors

[Laurentiu Palcu]

This adds support for starting/stopping streaming from connected
sensors as well. The user can also switch over to testing the test
pattern by configuring the routes accordingly.

Use the 'maxim,operation-mode' DT setting to allow the user to select
which operation mode the deserializer should run in, though only
tunneling mode is supported currently.

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
---
 drivers/staging/media/max96712/max96712.c | 426 +++++++++++++++++++++-
 1 file changed, 418 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
index 0112052171b06..b4c3d1d3c9539 100644
--- a/drivers/staging/media/max96712/max96712.c
+++ b/drivers/staging/media/max96712/max96712.c
@@ -4,6 +4,7 @@
  *
  * Copyright (C) 2021 Renesas Electronics Corporation
  * Copyright (C) 2021 Niklas Söderlund
+ * Copyright 2025 NXP
  */
 
 #include <linux/delay.h>
@@ -30,6 +31,12 @@
 #define   DIS_REM_CC_C_SHIFT				4
 #define   DIS_REM_CC_D_MASK				GENMASK(7, 6)
 #define   DIS_REM_CC_D_SHIFT				6
+#define MAX96712_DEV_CTRL12				CCI_REG8(0x000a)
+#define   LOCKED_B					BIT(3)
+#define MAX96712_DEV_CTRL13				CCI_REG8(0x000b)
+#define   LOCKED_C					BIT(3)
+#define MAX96712_DEV_CTRL14				CCI_REG8(0x000c)
+#define   LOCKED_D					BIT(3)
 
 /* TOP_CTRL */
 #define MAX96712_DEBUG_EXTRA_REG			CCI_REG8(0x0009)
@@ -37,6 +44,11 @@
 #define   DEBUG_EXTRA_PCLK_75MHZ			0x01
 #define MAX96724_TOP_CTRL_PWR1				CCI_REG8(0x0013)
 #define   RESET_ALL					BIT(6)
+#define MAX96712_TOP_CTRL_CTRL3				CCI_REG8(0x001a)
+#define   LOCK_PIN					BIT(0)
+#define   CMU_LOCKED					BIT(1)
+#define   ERROR						BIT(2)
+#define   LOCKED_A					BIT(3)
 
 /* BACKTOP0 */
 #define MAX96712_BACKTOP0_12				CCI_REG8(0x040b)
@@ -110,6 +122,15 @@
 #define   T_T3_PREP_86_7NS				3
 #define   T_T3_POST_MASK				GENMASK(6, 2)
 #define   T_T3_POST_SHIFT				2
+#define MAX96712_MIPI_PHY_MIPI_CTRL_SEL			CCI_REG8(0x08ca)
+#define   MIPI_CTRL_SEL_0_MASK				GENMASK(1, 0)
+#define   MIPI_CTRL_SEL_0_SHIFT				0
+#define   MIPI_CTRL_SEL_1_MASK				GENMASK(3, 2)
+#define   MIPI_CTRL_SEL_1_SHIFT				2
+#define   MIPI_CTRL_SEL_2_MASK				GENMASK(5, 4)
+#define   MIPI_CTRL_SEL_2_SHIFT				4
+#define   MIPI_CTRL_SEL_3_MASK				GENMASK(7, 6)
+#define   MIPI_CTRL_SEL_3_SHIFT				6
 
 /* MIPI_TX: 0 <= phy < 4 */
 #define MAX96712_MIPI_TX_DESKEW_INIT(phy)		CCI_REG8(0x0903 + (phy) * 0x40)
@@ -123,6 +144,22 @@
 #define   CSI2_CPHY_EN					BIT(5)
 #define   CSI2_LANE_CNT_MASK				GENMASK(7, 6)
 #define   CSI2_LANE_CNT_SHIFT				6
+#define MAX96712_MIPI_TX_54(phy)			CCI_REG8(0x0936 + (phy) * 0x40)
+#define   TUN_EN					BIT(0)
+#define   DESKEW_TUN_SRC_MASK				GENMASK(2, 1)
+#define   DESKEW_TUN_SRC_SHIFT				1
+#define   TUN_SER_LANE_NUM_MASK				GENMASK(4, 3)
+#define   TUN_SER_LANE_NUM_SHIFT			3
+#define   DESKEW_TUN_MASK				GENMASK(6, 5)
+#define   DESKEW_TUN_SHIFT				5
+#define   TUN_NO_CORR					BIT(7)
+#define MAX96712_MIPI_TX_57(phy)			CCI_REG8(0x0939 + (phy) * 0x40)
+#define   TUN_DPHY_TO_CPHY_CONV_OVRD			BIT(1)
+#define   TUN_DPHY_TO_CPHY_CONV				BIT(2)
+#define   TUN_DEST_MASK					GENMASK(5, 4)
+#define   TUN_DEST_SHIFT				4
+#define   DIS_AUTO_TUN_DET				BIT(6)
+#define   DIS_AUTO_SER_LANE_DET				BIT(7)
 
 /* GPIO_A: 0 <= gpio < 11 */
 #define MAX96712_GPIO_A_A(gpio)				CCI_REG8(0x0300 + (gpio) * 0x03)
@@ -241,6 +278,16 @@ struct max96712_rx_port {
 	struct fwnode_handle *fwnode;
 };
 
+struct max96712_asc {
+	struct v4l2_async_connection base;
+	struct max96712_rx_port *rx_port;
+};
+
+enum max96712_operation_mode {
+	MAX96712_TUNNEL_MODE,
+	MAX96712_PIXEL_MODE,
+};
+
 struct max96712_priv {
 	struct i2c_client *client;
 	struct regmap *regmap;
@@ -253,6 +300,8 @@ struct max96712_priv {
 
 	const struct max96712_info *info;
 
+	enum max96712_operation_mode operation_mode;
+
 	bool cphy;
 	struct v4l2_mbus_config_mipi_csi2 mipi;
 	s64 link_freq;
@@ -260,12 +309,15 @@ struct max96712_priv {
 	struct v4l2_subdev sd;
 	struct v4l2_ctrl_handler ctrl_handler;
 	struct media_pad pads[MAX96712_MAX_PORTS];
+	struct v4l2_async_notifier notifier;
+	u32 enabled_streams;
 
 	struct max96712_rx_port rx_ports[MAX96712_MAX_RX_PORTS];
 	unsigned int rx_port_mask;
 	unsigned int n_rx_ports;
 
 	enum max96712_pattern pattern;
+	bool vpg_started;
 
 	struct max96712_fsync fsync;
 	struct v4l2_fract interval;
@@ -382,6 +434,17 @@ static inline bool max96712_pad_is_source(u32 pad)
 	return pad >= MAX96712_FIRST_SOURCE_PAD && pad < MAX96712_VPG_PAD;
 }
 
+static int max96712_read(struct max96712_priv *priv, unsigned int reg, u64 *val)
+{
+	int ret;
+
+	ret = cci_read(priv->regmap, reg, val, NULL);
+	if (ret)
+		dev_err(&priv->client->dev, "read 0x%04x failed\n", reg);
+
+	return ret;
+}
+
 static int max96712_write(struct max96712_priv *priv, unsigned int reg, u64 val)
 {
 	int ret;
@@ -422,6 +485,14 @@ static void max96712_mipi_enable(struct max96712_priv *priv, bool enable)
 	}
 }
 
+static void max96712_tunneling_enable(struct max96712_priv *priv, bool enable)
+{
+	int i;
+
+	for (i = 0; i < 4; i++)
+		max96712_update_bits(priv, MAX96712_MIPI_TX_54(i), TUN_EN, enable ? TUN_EN : 0);
+}
+
 static void max96712_mipi_configure(struct max96712_priv *priv)
 {
 	unsigned int i;
@@ -485,14 +556,26 @@ static void max96712_mipi_configure(struct max96712_priv *priv)
 				     PERIODIC_DESKEW_CALIBRATION_EN, auto_deskew_calib_en);
 	}
 
+	if (priv->operation_mode == MAX96712_TUNNEL_MODE) {
+		int i;
+		/*
+		 * Disable tunnel auto-detection for all phys, will enable tunnelling
+		 * explicitly when needed.
+		 */
+		for (i = 0; i < 4; i++)
+			max96712_update_bits(priv, MAX96712_MIPI_TX_57(i),
+					     DIS_AUTO_TUN_DET, DIS_AUTO_TUN_DET);
+	}
+
 	/* Enable PHY0 and PHY1 */
 	max96712_update_bits(priv, MAX96712_MIPI_PHY_2, PHY_STDBY_N_MASK, PHY0_EN | PHY1_EN);
 }
 
-static void max96712_pattern_enable(struct max96712_priv *priv, struct v4l2_subdev_state *state,
-				    bool enable)
+static int max96712_pattern_enable(struct max96712_priv *priv, struct v4l2_subdev_state *state,
+				   bool enable)
 {
 	struct v4l2_mbus_framefmt *fmt = v4l2_subdev_state_get_format(state, MAX96712_VPG_PAD);
+	struct device *dev = &priv->client->dev;
 
 	const u32 h_active = fmt->width;
 	const u32 h_fp = 88;
@@ -506,9 +589,16 @@ static void max96712_pattern_enable(struct max96712_priv *priv, struct v4l2_subd
 	const u32 v_bp = 36;
 	const u32 v_tot = v_active + v_fp + v_sw + v_bp;
 
+	priv->vpg_started = enable;
+
 	if (!enable) {
 		max96712_write(priv, MAX96712_VRX_PATGEN_1, 0x00);
-		return;
+		return 0;
+	}
+
+	if (priv->enabled_streams) {
+		dev_err(dev, "Cannot enable VPG when other streams are enabled.\n");
+		return -EINVAL;
 	}
 
 	max96712_write(priv, MAX96712_DEBUG_EXTRA_REG, DEBUG_EXTRA_PCLK_75MHZ);
@@ -553,14 +643,44 @@ static void max96712_pattern_enable(struct max96712_priv *priv, struct v4l2_subd
 		/* Generate gradient pattern. */
 		max96712_write(priv, MAX96712_VRX_PATGEN_1, PATGEN_MODE_GRADIENT);
 	}
+
+	return 0;
+}
+
+static u8 max96712_get_link_status(struct max96712_priv *priv)
+{
+	u32 link_lock_addr[4] = {
+		MAX96712_TOP_CTRL_CTRL3,
+		MAX96712_DEV_CTRL12,
+		MAX96712_DEV_CTRL13,
+		MAX96712_DEV_CTRL14
+	};
+	int nport;
+	u8 link_status_mask = 0;
+
+	for (nport = 0; nport < MAX96712_MAX_RX_PORTS; nport++) {
+		u64 reg_val = 0;
+
+		max96712_read(priv, link_lock_addr[nport], &reg_val);
+
+		link_status_mask |= reg_val & BIT(3) ? (1 << nport) : 0;
+	}
+
+	return link_status_mask;
 }
 
-static int __maybe_unused max96712_fsync_set(struct max96712_priv *priv)
+static int max96712_fsync_enable(struct max96712_priv *priv, bool enable)
 {
 	u32 fsync;
 	int ret;
 	u8 mode_map[4] = {3, 0, 1, 2};
 
+	if (!enable) {
+		max96712_update_bits(priv, MAX96712_FSYNC_0,
+				     FSYNC_MODE_MASK, 0x3 << FSYNC_MODE_SHIFT);
+		return 0;
+	}
+
 	if (priv->fsync.mode == MAX96712_FSYNC_OFF)
 		return 0;
 
@@ -739,14 +859,117 @@ static int max96712_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
 	return ret;
 }
 
+static struct v4l2_subdev *max96712_xlate_streams(struct max96712_priv *priv,
+						  struct v4l2_subdev_state *state, u32 src_pad,
+						  u64 src_streams, u32 sink_pad, u64 *sink_streams,
+						  u32 *remote_pad)
+{
+	struct device *dev = &priv->client->dev;
+	u64 streams;
+	struct v4l2_subdev *remote_sd;
+	struct media_pad *pad;
+
+	streams = v4l2_subdev_state_xlate_streams(state, src_pad, sink_pad, &src_streams);
+	if (!streams)
+		dev_dbg(dev, "no streams found on sink pad\n");
+
+	pad = media_pad_remote_pad_first(&priv->pads[sink_pad]);
+	if (!pad) {
+		dev_dbg(dev, "no remote pad found for sink pad\n");
+		return ERR_PTR(-EPIPE);
+	}
+
+	remote_sd = media_entity_to_v4l2_subdev(pad->entity);
+	if (!remote_sd) {
+		dev_dbg(dev, "no entity connected to CSI2 input\n");
+		return ERR_PTR(-EPIPE);
+	}
+
+	*sink_streams = streams;
+	*remote_pad = pad->index;
+
+	return remote_sd;
+}
+
+static int max96712_enable_remote_stream(struct max96712_priv *priv,
+					 struct v4l2_subdev_state *state,
+					 u32 source_pad, u32 stream, u32 sink_pad,
+					 bool enable)
+{
+	struct device *dev = &priv->client->dev;
+	struct v4l2_subdev *remote_sd;
+	u64 sink_streams = 0;
+	u32 remote_pad = 0;
+	int ret = 0;
+
+	if (enable && priv->vpg_started) {
+		dev_err(dev, "Cannot enable remote streams while VPG is enabled.\n");
+		return -EINVAL;
+	}
+
+	remote_sd = max96712_xlate_streams(priv, state, source_pad, BIT(stream), sink_pad,
+					   &sink_streams, &remote_pad);
+	if (IS_ERR(remote_sd))
+		return PTR_ERR(remote_sd);
+
+	ret = enable ? v4l2_subdev_enable_streams(remote_sd, remote_pad, 0x1) :
+		       v4l2_subdev_disable_streams(remote_sd, remote_pad, 0x1);
+
+	if (ret)
+		dev_err(&priv->client->dev, "failed to %s streams 0x%llx on '%s':%u: %d\n",
+			enable ? "enable" : "disable",
+			sink_streams, remote_sd->name, remote_pad, ret);
+
+	return ret;
+}
+
 static int max96712_enable_streams(struct v4l2_subdev *sd,
 				   struct v4l2_subdev_state *state,
 				   u32 source_pad, u64 streams_mask)
 {
 	struct max96712_priv *priv = v4l2_get_subdevdata(sd);
+	u64 sources_mask = streams_mask;
+	u32 sink_pad, sink_stream;
+	int ret = 0;
+
+	if (!priv->enabled_streams)
+		max96712_mipi_enable(priv, true);
+
+	while (true) {
+		int pos = ffs(sources_mask) - 1;
 
-	max96712_pattern_enable(priv, state, true);
-	max96712_mipi_enable(priv, true);
+		if (pos == -1)
+			break;
+
+		ret = v4l2_subdev_routing_find_opposite_end(&state->routing,
+							    source_pad, pos,
+							    &sink_pad, &sink_stream);
+		if (ret)
+			return ret;
+
+		if (sink_pad == MAX96712_VPG_PAD && sink_stream == 0) {
+			/* need to have tunneling disabled for VPG to work */
+			max96712_tunneling_enable(priv, false);
+
+			ret = max96712_pattern_enable(priv, state, true);
+		} else {
+			if (!priv->enabled_streams) {
+				max96712_fsync_enable(priv, true);
+				if (priv->operation_mode == MAX96712_TUNNEL_MODE)
+					max96712_tunneling_enable(priv, true);
+			}
+
+			ret = max96712_enable_remote_stream(priv, state, source_pad, pos,
+							    sink_pad, true);
+		}
+
+		if (ret)
+			return ret;
+
+		sources_mask &= ~BIT(pos);
+	}
+
+	priv->enabled_streams |= streams_mask;
 
 	return 0;
 }
@@ -756,9 +979,42 @@ static int max96712_disable_streams(struct v4l2_subdev *sd,
 				    u32 source_pad, u64 streams_mask)
 {
 	struct max96712_priv *priv = v4l2_get_subdevdata(sd);
+	u64 sources_mask = streams_mask;
+	u32 sink_pad, sink_stream;
+	int ret = 0;
 
-	max96712_mipi_enable(priv, false);
-	max96712_pattern_enable(priv, state, false);
+	while (true) {
+		int pos = ffs(sources_mask) - 1;
+
+		if (pos == -1)
+			break;
+
+		ret = v4l2_subdev_routing_find_opposite_end(&state->routing,
+							    source_pad, pos,
+							    &sink_pad, &sink_stream);
+		if (ret)
+			return ret;
+
+		max96712_update_bits(priv, MAX96712_MIPI_PHY_0, FORCE_CSI_OUT_EN, 0x00);
+
+		if (sink_pad == MAX96712_VPG_PAD && sink_stream == 0)
+			ret = max96712_pattern_enable(priv, state, false);
+		else
+			ret = max96712_enable_remote_stream(priv, state, source_pad, pos,
+							    sink_pad, false);
+
+		if (ret)
+			return ret;
+
+		sources_mask &= ~BIT(pos);
+	}
+
+	priv->enabled_streams &= ~streams_mask;
+
+	if (!priv->enabled_streams) {
+		max96712_fsync_enable(priv, false);
+		max96712_mipi_enable(priv, false);
+	}
 
 	return 0;
 }
@@ -842,6 +1098,104 @@ static int max96712_set_fmt(struct v4l2_subdev *sd,
 	return 0;
 }
 
+#define to_index(priv, rx_port) ((rx_port) - &(priv)->rx_ports[0])
+
+static int max96712_notify_bound(struct v4l2_async_notifier *notifier,
+				 struct v4l2_subdev *subdev,
+				 struct v4l2_async_connection *asc)
+{
+	struct max96712_priv *priv = container_of(notifier->sd, struct max96712_priv, sd);
+	struct max96712_asc *async_conn = container_of(asc, struct max96712_asc, base);
+	struct max96712_rx_port *rx_port = async_conn->rx_port;
+	unsigned int index = to_index(priv, rx_port);
+	struct device *dev = &priv->client->dev;
+	unsigned int src_pad;
+	int ret;
+
+	ret = media_entity_get_fwnode_pad(&subdev->entity, rx_port->fwnode, MEDIA_PAD_FL_SOURCE);
+	if (ret < 0) {
+		dev_err(dev, "Failed to find pad for %s\n", subdev->name);
+		return ret;
+	}
+
+	rx_port->sd = subdev;
+	src_pad = ret;
+
+	ret = media_create_pad_link(&rx_port->sd->entity, src_pad, &priv->sd.entity, index, 0);
+	if (ret) {
+		dev_err(dev, "Unable to link %s:%u -> %s:%u\n",
+			rx_port->sd->name, src_pad, priv->sd.name, index);
+		return ret;
+	}
+
+	dev_dbg(dev, "Bound %s pad: %u on index %u\n", subdev->name, src_pad, index);
+
+	return 0;
+}
+
+static void max96712_notify_unbind(struct v4l2_async_notifier *notifier,
+				   struct v4l2_subdev *subdev,
+				   struct v4l2_async_connection *asc)
+{
+	struct max96712_asc *async_conn = container_of(asc, struct max96712_asc, base);
+	struct max96712_rx_port *rx_port = async_conn->rx_port;
+
+	rx_port->sd = NULL;
+}
+
+static const struct v4l2_async_notifier_operations max96724_notify_ops = {
+	.bound = max96712_notify_bound,
+	.unbind = max96712_notify_unbind,
+};
+
+static int max96712_v4l2_notifier_register(struct max96712_priv *priv)
+{
+	int i, ret;
+	struct device *dev = &priv->client->dev;
+	struct max96712_rx_port *rx_port = NULL;
+	u32 rx_port_mask = priv->rx_port_mask;
+
+	if (!priv->n_rx_ports)
+		return 0;
+
+	v4l2_async_subdev_nf_init(&priv->notifier, &priv->sd);
+
+	while (true) {
+		int pos = ffs(rx_port_mask) - 1;
+		struct max96712_asc *asc;
+
+		if (pos == -1)
+			break;
+
+		rx_port = &priv->rx_ports[pos];
+		rx_port_mask &= ~BIT(pos);
+
+		if (!rx_port->fwnode)
+			continue;
+
+		asc = v4l2_async_nf_add_fwnode(&priv->notifier, rx_port->fwnode,
+					       struct max96712_asc);
+		if (IS_ERR(asc)) {
+			dev_err(dev, "Failed to add subdev for source %u: %ld", i, PTR_ERR(asc));
+			v4l2_async_nf_cleanup(&priv->notifier);
+			return PTR_ERR(asc);
+		}
+
+		asc->rx_port = rx_port;
+	}
+
+	priv->notifier.ops = &max96724_notify_ops;
+
+	ret = v4l2_async_nf_register(&priv->notifier);
+	if (ret) {
+		dev_err(dev, "Failed to register subdev_notifier");
+		v4l2_async_nf_cleanup(&priv->notifier);
+		return ret;
+	}
+
+	return 0;
+}
+
 static int max96712_init_state(struct v4l2_subdev *sd,
 			       struct v4l2_subdev_state *state)
 {
@@ -862,6 +1216,37 @@ static int max96712_init_state(struct v4l2_subdev *sd,
 	return _max96712_set_routing(sd, state, &routing);
 }
 
+static int max96712_log_status(struct v4l2_subdev *sd)
+{
+	struct max96712_priv *priv = container_of(sd, struct max96712_priv, sd);
+	struct device *dev = &priv->client->dev;
+	u8 gmsl_link_status_mask;
+	char hdr[64];
+	int nport;
+
+	gmsl_link_status_mask = max96712_get_link_status(priv);
+
+	dev_info(dev, "Deserializer status:\n");
+
+	dev_info(dev, "RX ports:\n");
+
+	for (nport = 0; nport < MAX96712_MAX_RX_PORTS; nport++) {
+		struct max96712_rx_port *rx_port = &priv->rx_ports[nport];
+
+		sprintf(hdr, "\t* RX %d:", nport);
+
+		if (!rx_port->fwnode) {
+			dev_info(dev, "%s Not Configured\n", hdr);
+			continue;
+		}
+
+		dev_info(dev, "%s Link %s\n", hdr,
+			 gmsl_link_status_mask & BIT(nport) ? "locked" : "not locked");
+	}
+
+	return 0;
+}
+
 static const struct v4l2_subdev_internal_ops max96712_internal_ops = {
 	.init_state = max96712_init_state,
 };
@@ -878,8 +1263,13 @@ static const struct v4l2_subdev_pad_ops max96712_pad_ops = {
 	.set_frame_interval	= max96712_set_frame_interval,
 };
 
+static const struct v4l2_subdev_core_ops max96712_subdev_core_ops = {
+	.log_status = max96712_log_status,
+};
+
 static const struct v4l2_subdev_ops max96712_subdev_ops = {
 	.video = &max96712_video_ops,
+	.core = &max96712_subdev_core_ops,
 	.pad = &max96712_pad_ops,
 };
 
@@ -907,6 +1297,10 @@ static const struct v4l2_ctrl_ops max96712_ctrl_ops = {
 	.s_ctrl = max96712_s_ctrl,
 };
 
+static const struct media_entity_operations max96712_v4l2_media_ops = {
+	.link_validate = v4l2_subdev_link_validate,
+};
+
 static int max96712_v4l2_register(struct max96712_priv *priv)
 {
 	struct v4l2_ctrl *link_freq_ctrl;
@@ -917,6 +1311,7 @@ static int max96712_v4l2_register(struct max96712_priv *priv)
 	v4l2_i2c_subdev_init(&priv->sd, priv->client, &max96712_subdev_ops);
 	priv->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_STREAMS;
 	priv->sd.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
+	priv->sd.entity.ops = &max96712_v4l2_media_ops;
 
 	v4l2_ctrl_handler_init(&priv->ctrl_handler, 2);
 
@@ -953,6 +1348,12 @@ static int max96712_v4l2_register(struct max96712_priv *priv)
 	if (ret)
 		goto error;
 
+	ret = max96712_v4l2_notifier_register(priv);
+	if (ret) {
+		dev_err(&priv->client->dev, "Unable to register v4l2 async notifiers\n");
+		goto error;
+	}
+
 	ret = v4l2_async_register_subdev(&priv->sd);
 	if (ret < 0) {
 		dev_err(&priv->client->dev, "Unable to register subdevice\n");
@@ -960,6 +1361,7 @@ static int max96712_v4l2_register(struct max96712_priv *priv)
 	}
 
 	return 0;
+
 error:
 	v4l2_ctrl_handler_free(&priv->ctrl_handler);
 
@@ -1223,6 +1625,14 @@ static int max96712_parse_dt(struct max96712_priv *priv)
 	int ret = 0, count;
 	u32 dt_val[3];
 
+	if (!fwnode_property_read_u32(dev_fwnode(dev), "maxim,operation-mode", &dt_val[0]))
+		priv->operation_mode = dt_val[0];
+
+	if (priv->operation_mode != MAX96712_TUNNEL_MODE) {
+		dev_err(dev, "Unsupported mode, only tunneling mode is supported currently.\n");
+		return -EINVAL;
+	}
+
 	count = fwnode_property_count_u32(dev_fwnode(dev), "maxim,fsync-config");
 	if (count > 0) {
 		ret = fwnode_property_read_u32_array(dev_fwnode(dev), "maxim,fsync-config",
-- 
2.44.1

Re: [RFC 00/12] staging: media: max96712: Add support for streams and multiple sensors

[Niklas Söderlund]

Hi Laurentiu,

Thanks for your work. I'm happy someone with a both GMSL cameras and a 
max96712 found time to work on this driver.

On 2025-01-31 18:33:54 +0200, Laurentiu Palcu wrote:
> Hi,
> 
> This series adds more functionality to the existing max96712 staging
> driver allowing multiple sensors to be connected through other
> compatible serializers. I tried to split the changes in smaller logical
> changes to make them easier to review while not altering the existing
> VPG functionality but I could squash all of them together if needed.

With this series and it's listed dependencies applied my CI tests using 
the VPG breaks. The controls to select test-pattern seems to be invalid,

    $ yavta --set-control '0x009f0903 0' /dev/v4l-subdev6
    Device /dev/v4l-subdev6 opened.
    unable to set control 0x009f0903: Permission denied (13).
    Unable to get format: Inappropriate ioctl for device (25).

    (/dev/v4l-subdev6 here is max96712 1-0049)

    $ yavta -c10 --file=/tmp/vin-capture/isp0-checkerboard-#.bin /dev/video0
    Device /dev/video0 opened.
    Device `R_Car_VIN' on `platform:e6ef0000.video' (driver 'rcar_vin') supports video, capture, without mplanes.
    Video format: ABGR32 (34325241) 1920x1080 (stride 7680) field none buffer size 8294400
    8 buffers requested.
    length: 8294400 offset: 0 timestamp type/source: mono/EoF
    Buffer 0/0 mapped at address 0xffffbe5d7000.
    length: 8294400 offset: 32768 timestamp type/source: mono/EoF
    Buffer 1/0 mapped at address 0xffffbddee000.
    length: 8294400 offset: 65536 timestamp type/source: mono/EoF
    Buffer 2/0 mapped at address 0xffffbd605000.
    length: 8294400 offset: 98304 timestamp type/source: mono/EoF
    Buffer 3/0 mapped at address 0xffffbce1c000.
    length: 8294400 offset: 131072 timestamp type/source: mono/EoF
    Buffer 4/0 mapped at address 0xffffbc633000.
    length: 8294400 offset: 163840 timestamp type/source: mono/EoF
    Buffer 5/0 mapped at address 0xffffbbe4a000.
    length: 8294400 offset: 196608 timestamp type/source: mono/EoF
    Buffer 6/0 mapped at address 0xffffbb661000.
    length: 8294400 offset: 229376 timestamp type/source: mono/EoF
    Buffer 7/0 mapped at address 0xffffbae78000.
    Unable to start streaming: Invalid argument (22).

I read in patch 12/12 "The user can also switch over to testing the test 
pattern by configuring the routes accordingly", but not how to do that 
to achieve the same functionality as the staging driver. Inspecting the 
media graph gives little help. Would it be possible to extend the cover 
letter with this information?

To be clear, I don't care about the change in behavior as this driver 
obviously primary aim should be to  support GMSL2 cameras, not 
test-pattern generation. It is important for me that it is possible to 
enable the test pattern generation $somehow at runtime (i.e. no DTS 
changes). As this is the only method I have to test a bunch of boards.

It would also be nice if the patterns generated (output frames) as 
closely as possible would resembles what is generated today. That way I 
don't have to alter my CI pipelines too much :-)

> 
> The series only supports tunneling mode and uses the first MIPI-CSI
> port. Support for more functionality can be added later, if needed.
> 
> I sent the set as a RFC because it depends on Sakari's pending internal
> pads patch which is needed if we want to have an elegant solution for
> allowing the user to switch between streaming from sensors or just
> video pattern generation.
> 
> Also, the set depends on my previous series which was not yet merged:
> https://patchwork.linuxtv.org/project/linux-media/list/?series=14255
> 
> Thanks,
> Laurentiu
> 
> Laurentiu Palcu (11):
>   dt-bindings: i2c: maxim,max96712: add a couple of new properties
>   staging: media: max96712: convert to using CCI register access helpers
>   staging: media: max96712: change DT parsing routine
>   staging: media: max96712: add link frequency V4L2 control
>   staging: media: max96712: add I2C mux support
>   staging: media: max96712: add support for streams
>   staging: media: max96712: allow enumerating MBUS codes
>   staging: media: max96712: add set_fmt routine
>   staging: media: max96712: add gpiochip functionality
>   staging: media: max96712: add fsync support
>   staging: media: max96712: allow streaming from connected sensors
> 
> Sakari Ailus (1):
>   media: mc: Add INTERNAL pad flag
> 
>  .../bindings/media/i2c/maxim,max96712.yaml    |   45 +
>  .../media/mediactl/media-types.rst            |    8 +
>  drivers/media/mc/mc-entity.c                  |   10 +-
>  drivers/staging/media/max96712/max96712.c     | 1406 +++++++++++++++--
>  include/uapi/linux/media.h                    |    1 +
>  5 files changed, 1352 insertions(+), 118 deletions(-)
> 
> -- 
> 2.44.1
> 

-- 
Kind Regards,
Niklas Söderlund

Re: [RFC 00/12] staging: media: max96712: Add support for streams and multiple sensors

[Laurentiu Palcu]

Hi Niklas,

Thanks for giving this series a test. More comments below.

On Tue, Feb 04, 2025 at 01:39:25PM +0100, Niklas Söderlund wrote:
> Hi Laurentiu,
> 
> Thanks for your work. I'm happy someone with a both GMSL cameras and a 
> max96712 found time to work on this driver.

I don't have a max96712 unfortunately. Our setup is based on max96724.

> 
> On 2025-01-31 18:33:54 +0200, Laurentiu Palcu wrote:
> > Hi,
> > 
> > This series adds more functionality to the existing max96712 staging
> > driver allowing multiple sensors to be connected through other
> > compatible serializers. I tried to split the changes in smaller logical
> > changes to make them easier to review while not altering the existing
> > VPG functionality but I could squash all of them together if needed.
> 
> With this series and it's listed dependencies applied my CI tests using 
> the VPG breaks. The controls to select test-pattern seems to be invalid,
> 
>     $ yavta --set-control '0x009f0903 0' /dev/v4l-subdev6
>     Device /dev/v4l-subdev6 opened.
>     unable to set control 0x009f0903: Permission denied (13).
>     Unable to get format: Inappropriate ioctl for device (25).

That's my bad... :/ I have never tried to change test patterns and I
didn't spot the bug. The below change should fix that:

diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
index b4c3d1d3c9539..6d6dea0a335c7 100644
--- a/drivers/staging/media/max96712/max96712.c
+++ b/drivers/staging/media/max96712/max96712.c
@@ -1315,10 +1315,10 @@ static int max96712_v4l2_register(struct max96712_priv *priv)

        v4l2_ctrl_handler_init(&priv->ctrl_handler, 2);

-       v4l2_ctrl_new_int_menu(&priv->ctrl_handler, NULL, V4L2_CID_LINK_FREQ,
+       link_freq_ctrl = v4l2_ctrl_new_int_menu(&priv->ctrl_handler, NULL, V4L2_CID_LINK_FREQ,
                               0, 0, &priv->link_freq);

-       link_freq_ctrl = v4l2_ctrl_new_std_menu_items(&priv->ctrl_handler, &max96712_ctrl_ops,
+       v4l2_ctrl_new_std_menu_items(&priv->ctrl_handler, &max96712_ctrl_ops,
                                                      V4L2_CID_TEST_PATTERN,
                                                      ARRAY_SIZE(max96712_test_pattern) - 1,
                                                      0, 0, max96712_test_pattern);

> 
>     (/dev/v4l-subdev6 here is max96712 1-0049)
> 
>     $ yavta -c10 --file=/tmp/vin-capture/isp0-checkerboard-#.bin /dev/video0
>     Device /dev/video0 opened.
>     Device `R_Car_VIN' on `platform:e6ef0000.video' (driver 'rcar_vin') supports video, capture, without mplanes.
>     Video format: ABGR32 (34325241) 1920x1080 (stride 7680) field none buffer size 8294400
>     8 buffers requested.
>     length: 8294400 offset: 0 timestamp type/source: mono/EoF
>     Buffer 0/0 mapped at address 0xffffbe5d7000.
>     length: 8294400 offset: 32768 timestamp type/source: mono/EoF
>     Buffer 1/0 mapped at address 0xffffbddee000.
>     length: 8294400 offset: 65536 timestamp type/source: mono/EoF
>     Buffer 2/0 mapped at address 0xffffbd605000.
>     length: 8294400 offset: 98304 timestamp type/source: mono/EoF
>     Buffer 3/0 mapped at address 0xffffbce1c000.
>     length: 8294400 offset: 131072 timestamp type/source: mono/EoF
>     Buffer 4/0 mapped at address 0xffffbc633000.
>     length: 8294400 offset: 163840 timestamp type/source: mono/EoF
>     Buffer 5/0 mapped at address 0xffffbbe4a000.
>     length: 8294400 offset: 196608 timestamp type/source: mono/EoF
>     Buffer 6/0 mapped at address 0xffffbb661000.
>     length: 8294400 offset: 229376 timestamp type/source: mono/EoF
>     Buffer 7/0 mapped at address 0xffffbae78000.
>     Unable to start streaming: Invalid argument (22).
> 
> I read in patch 12/12 "The user can also switch over to testing the test 
> pattern by configuring the routes accordingly", but not how to do that 
> to achieve the same functionality as the staging driver. Inspecting the 
> media graph gives little help. Would it be possible to extend the cover 
> letter with this information?

I can do that, sure, but routing setup depends on the board you test on... :/
Basically, the deserializer media node has 6 pads now. Pad 4 is first CSI
output port and pad 6 is the internal VPG source pad. By default, the route
from internal VPG pad to pad 4 is active. So, you shouldn't need to set any
routes on max96712 node for testing VPG. This is how it looks like:

- entity 120: max96712 2-0027 (7 pads, 5 links, 1 route)      
              type V4L2 subdev subtype Unknown flags 0
              device node name /dev/v4l-subdev11                                                        
        routes:                          
                6/0 -> 4/0 [ACTIVE]
        pad0: Sink                                                                                      
                <- "max96717 8-0040":1 []                                                               
        pad1: Sink                              
                <- "max96717 9-0040":1 []                                                               
        pad2: Sink                                                                                                                                                                                              
                <- "max96717 10-0040":1 []          
        pad3: Sink                                                                                      
                <- "max96717 14-0040":1 []                                                              
        pad4: Source                            
                [stream:0 fmt:RGB888_1X24/1920x1080 field:none colorspace:srgb]
                -> "csidev-4ad30000.csi":0 []
        pad5: Source
        pad6: Sink, Internal
                [stream:0 fmt:RGB888_1X24/1920x1080 field:none colorspace:srgb]

Below is the test script I use to set the routings and links for all the
pipeline in order to test VPG. I'm testing on an i.MX95 board.

./media-ctl -r
./media-ctl -d /dev/media0 -R '"max96712 2-0027" [6/0 -> 4/0 [1]]'
./media-ctl -d /dev/media0 --set-v4l2 '"max96712 2-0027":6/0 [fmt:RGB24/1920x1080 field:none]'
./media-ctl -d /dev/media0 -R '"csidev-4ad30000.csi" [0/0 -> 1/0 [1]]'
./media-ctl -d /dev/media0 --set-v4l2 '"csidev-4ad30000.csi":0/0 [fmt:RGB24/1920x1080 field:none]'
./media-ctl -d /dev/media0 -R '"4ac10000.syscon:formatter@20" [0/0 -> 1/0 [1]]'
./media-ctl -d /dev/media0 --set-v4l2 '"4ac10000.syscon:formatter@20":0/0 [fmt:RGB24/1920x1080 field:none]'
./media-ctl -d /dev/media0 -R '"crossbar" [2/0 -> 7/0 [1]]'
./media-ctl -d /dev/media0 --set-v4l2 '"crossbar":2/0 [fmt:RGB24/1920x1080 field:none]'
./media-ctl -d /dev/media0 --set-v4l2 '"mxc_isi.2":0/0 [fmt:RGB24/1920x1080 field:none]'
./media-ctl -d /dev/media0 -l "'max96712 2-0027':4 -> 'csidev-4ad30000.csi':0 [1]"

./v4l2-ctl --device /dev/video2 --set-fmt-video=width=1920,height=1080,pixelformat=RGB3 --stream-mmap --stream-count=100

However, I suspect that, in your case, the downstream drivers do not support
streams and streaming cannot start. But I might be wrong though...

> 
> To be clear, I don't care about the change in behavior as this driver 
> obviously primary aim should be to  support GMSL2 cameras, not 
> test-pattern generation. It is important for me that it is possible to 
> enable the test pattern generation $somehow at runtime (i.e. no DTS 
> changes). As this is the only method I have to test a bunch of boards.

That's the idea. I can switch between capturing from sensors and generating the
test pattern at runtime. I don't do any changes in the DTB. However, I believe
the downstream devices need to support streams as well in order to work.

> 
> It would also be nice if the patterns generated (output frames) as 
> closely as possible would resembles what is generated today. That way I 
> don't have to alter my CI pipelines too much :-)

I didn't touch the pattern generation part at all. From what I can see, it
looks the same.

Thanks,
Laurentiu

> 
> > 
> > The series only supports tunneling mode and uses the first MIPI-CSI
> > port. Support for more functionality can be added later, if needed.
> > 
> > I sent the set as a RFC because it depends on Sakari's pending internal
> > pads patch which is needed if we want to have an elegant solution for
> > allowing the user to switch between streaming from sensors or just
> > video pattern generation.
> > 
> > Also, the set depends on my previous series which was not yet merged:
> > https://patchwork.linuxtv.org/project/linux-media/list/?series=14255
> > 
> > Thanks,
> > Laurentiu
> > 
> > Laurentiu Palcu (11):
> >   dt-bindings: i2c: maxim,max96712: add a couple of new properties
> >   staging: media: max96712: convert to using CCI register access helpers
> >   staging: media: max96712: change DT parsing routine
> >   staging: media: max96712: add link frequency V4L2 control
> >   staging: media: max96712: add I2C mux support
> >   staging: media: max96712: add support for streams
> >   staging: media: max96712: allow enumerating MBUS codes
> >   staging: media: max96712: add set_fmt routine
> >   staging: media: max96712: add gpiochip functionality
> >   staging: media: max96712: add fsync support
> >   staging: media: max96712: allow streaming from connected sensors
> > 
> > Sakari Ailus (1):
> >   media: mc: Add INTERNAL pad flag
> > 
> >  .../bindings/media/i2c/maxim,max96712.yaml    |   45 +
> >  .../media/mediactl/media-types.rst            |    8 +
> >  drivers/media/mc/mc-entity.c                  |   10 +-
> >  drivers/staging/media/max96712/max96712.c     | 1406 +++++++++++++++--
> >  include/uapi/linux/media.h                    |    1 +
> >  5 files changed, 1352 insertions(+), 118 deletions(-)
> > 
> > -- 
> > 2.44.1
> > 
> 
> -- 
> Kind Regards,
> Niklas Söderlund