[PATCH v6 0/4] media: i2c: ds90ub953: Add back channel GPIO support

[PATCH v6 0/4] media: i2c: ds90ub953: Add back channel GPIO support

[Guoniu Zhou]

Add back channel GPIO support.
  dt-bindings: media: ti,ds90ub953: Add support for remote GPIO data source
  media: i2c: ds90ub953: Add back channel GPIO support

Use helper macro to simplify code.
  media: i2c: ds90ub953: use devm_mutex_init() to simplify code
  media: i2c: ds90ub953: use guard() to simplify code

Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
---
Changes in v6:
- Rebased to latest media/next
- Replaced label "err" with "out"
- Changed approach from extending GPIO range (v5) to using a custom GPIO
  flag (GPIO_DATA_SOURCE_REMOTE) as suggested by the driver maintainer
- Dropped Reviewed-by tag from Rob Herring due to significant binding change
- Added GPIO_DATA_SOURCE_REMOTE flag to dt-bindings/gpio/gpio.h
- Updated dt-bindings documentation accordingly
- Added Reviewed-by tag from Tomi Valkeinen
- Detailed changes can be found in each patch's changelog
- Link to v5: https://lore.kernel.org/r/20260228-ds90ub953-v5-0-056cf07cc8f1@nxp.com

Changes in v5:
- Improve the description for "#gpio-cells" as commented by Conor.
- Detail change see each patches's change log.
- Link to v4: https://lore.kernel.org/r/20260227-ds90ub953-v4-0-dea596205f9e@nxp.com

Changes in v4:
- Rebase to 7.0-rc1
- Use folder block instead of literal block for #gpio-cell property description. 
- Only log GPIO 0-3 stats since remote GPIO 4-7 reuse GPIO 0-3 pins.
- Link to v3: https://lore.kernel.org/r/20250916-ds90ub953-v3-0-bb7bc6545da7@nxp.com

Changes in v3:
- Make GPIO range from 0-3 to 0-7 to support GPIO data from remote
  compatible deserializer suggested by Rob instead of adding third
  cell for GPIO controller.
- Update driver to expand GPIO range.
- Link to v2: https://lore.kernel.org/r/20250911-ds90ub953-v2-0-03ee76eb6b59@nxp.com

Changes in v2:
- Remove new property ti,gpio-data added in patch 1.
- Add third cell for GPIO controller to select GPIO output source.
- Move PTR_ERR() in dev_err_probe();
- Parse gpio third cell to select which GPIO pin the data from remote compatible deserializer.
- Link to v1: https://lore.kernel.org/r/20250910-ds90ub953-v1-0-a7813ffbdf11@nxp.com

---
Guoniu Zhou (4):
      dt-bindings: media: ti,ds90ub953: Add support for remote GPIO data source
      media: i2c: ds90ub953: Add back channel GPIO support
      media: i2c: ds90ub953: use devm_mutex_init() to simplify code
      media: i2c: ds90ub953: use guard() to simplify code

 .../bindings/media/i2c/ti,ds90ub953.yaml           |   6 +-
 drivers/media/i2c/ds90ub953.c                      | 118 +++++++++++++--------
 include/dt-bindings/gpio/gpio.h                    |   8 ++
 3 files changed, 83 insertions(+), 49 deletions(-)
---
base-commit: 4fbeef21f5387234111b5d52924e77757626faa5
change-id: 20250901-ds90ub953-168628c53a00

Best regards,
-- 
Guoniu Zhou <guoniu.zhou@oss.nxp.com>

[PATCH v6 1/4] dt-bindings: media: ti,ds90ub953: Add support for remote GPIO data source

[Guoniu Zhou]

From: Guoniu Zhou <guoniu.zhou@nxp.com>

The DS90UB953 supports four pins, GPIO0 through GPIO3. When enabled as an
output, it can be programed to output local data or remote data coming
from the remote compatible deserializer.

Add GPIO flag in second cell to select remote GPIO data source.

Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
---
Changes in v6:
- Added GPIO_DATA_SOURCE_REMOTE flag to dt-bindings/gpio/gpio.h
- Updated dt-bindings documentation accordingly
- Dropped Reviewed-by tag from Rob Herring due to significant binding change

Changes in v5:
- Improve the description for "#gpio-cells" as commented by Conor.

Changes in v4:
- Use folder block instead of literal block for #gpio-cell property description.

Changes in v3:
- Make GPIO range from 0-3 to 0-7 to support GPIO data from remote
  compatible deserializer suggested by Rob instead of adding third
  cell for GPIO controller.

Changes in v2:
- Remove new property ti,gpio-data
- Add third cell for GPIO controller to select GPIO output source.
---
 Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml | 6 ++++--
 include/dt-bindings/gpio/gpio.h                               | 8 ++++++++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
index 2e129bf573b7..da63771bc236 100644
--- a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
@@ -21,8 +21,10 @@ properties:
   '#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, 3].
+      First cell is the GPIO pin number (0-3) and the second cell is used
+      to specify flags. See <dt-bindings/gpio/gpio.h> for available flags
+      including GPIO_DATA_SOURCE_REMOTE for remote GPIO data source.
+      Flags can be OR'd together.
 
   gpio-controller: true
 
diff --git a/include/dt-bindings/gpio/gpio.h b/include/dt-bindings/gpio/gpio.h
index b5d531237448..d04a494d96ad 100644
--- a/include/dt-bindings/gpio/gpio.h
+++ b/include/dt-bindings/gpio/gpio.h
@@ -42,4 +42,12 @@
 /* Bit 6 express pull disable */
 #define GPIO_PULL_DISABLE 64
 
+/*
+ * Bit 24 indicates the GPIO data source is from a remote device.
+ * This is used in serializer/deserializer setups where the GPIO pin
+ * on the local device (e.g., TI DS90UB953 serializer) reflects the
+ * state of a GPIO on the remote device (e.g., TI DS90UB960 deserializer).
+ */
+#define GPIO_DATA_SOURCE_REMOTE		0x01000000
+
 #endif

-- 
2.34.1

[PATCH v6 2/4] media: i2c: ds90ub953: Add back channel GPIO support

[Guoniu Zhou]

From: Guoniu Zhou <guoniu.zhou@nxp.com>

The ds90ub953 supports GPIO0 through GPIO3. When enabled as an output,
each GPIO pin can be programed to output remote data coming from the
compatible deserializer using the register LOCAL_GPIO_DATA[7:4] field.

Add back channel GPIO support by parsing flags from device tree.

Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
---
Changes in v6:
- Changed approach from extending GPIO range (v5) to using a custom GPIO
  flag (GPIO_DATA_SOURCE_REMOTE) as suggested by the driver maintainer

Changes in v4:
- Only log GPIO 0-3 stats since remote GPIO 4-7 reuse GPIO 0-3 pins.

Changes in v3:
- Update driver to expand GPIO range.

Changes in v2:
- Parse gpio third cell to select which GPIO pin the data from remote
  compatible deserializer.
---
 drivers/media/i2c/ds90ub953.c | 51 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 45 insertions(+), 6 deletions(-)

diff --git a/drivers/media/i2c/ds90ub953.c b/drivers/media/i2c/ds90ub953.c
index a8ab67f4137f..da63dcfbbbc3 100644
--- a/drivers/media/i2c/ds90ub953.c
+++ b/drivers/media/i2c/ds90ub953.c
@@ -27,6 +27,8 @@
 #include <media/v4l2-mediabus.h>
 #include <media/v4l2-subdev.h>
 
+#include <dt-bindings/gpio/gpio.h>
+
 #include "ds90ub953.h"
 
 #define UB953_PAD_SINK			0
@@ -71,6 +73,7 @@ struct ub953_data {
 	bool			non_continous_clk;
 
 	struct gpio_chip	gpio_chip;
+	u32			gpio_flags[UB953_NUM_GPIOS];
 
 	struct v4l2_subdev	sd;
 	struct media_pad	pads[2];
@@ -258,6 +261,12 @@ static int ub953_write_ind(struct ub953_data *priv, u8 block, u8 reg, u8 val,
 /*
  * GPIO chip
  */
+
+static inline bool ub953_gpio_is_remote(unsigned int flag)
+{
+	return !!(flag & GPIO_DATA_SOURCE_REMOTE);
+}
+
 static int ub953_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
 {
 	struct ub953_data *priv = gpiochip_get_data(gc);
@@ -288,13 +297,23 @@ static int ub953_gpio_direction_out(struct gpio_chip *gc, unsigned int offset,
 				    int value)
 {
 	struct ub953_data *priv = gpiochip_get_data(gc);
+	bool is_remote = ub953_gpio_is_remote(priv->gpio_flags[offset]);
+	unsigned int mask, val;
 	int ret;
 
-	ret = regmap_update_bits(priv->regmap, UB953_REG_LOCAL_GPIO_DATA,
-				 UB953_REG_LOCAL_GPIO_DATA_GPIO_OUT_SRC(offset),
-				 value ? UB953_REG_LOCAL_GPIO_DATA_GPIO_OUT_SRC(offset) :
-					 0);
+	mask = UB953_REG_LOCAL_GPIO_DATA_GPIO_OUT_SRC(offset) |
+	       UB953_REG_LOCAL_GPIO_DATA_GPIO_RMTEN(offset);
 
+	if (is_remote) {
+		/* Enable remote deserializer GPIO data on local GPIO */
+		val = UB953_REG_LOCAL_GPIO_DATA_GPIO_RMTEN(offset);
+	} else {
+		/* Set output value on local GPIO and disable remote mode */
+		val = value ? UB953_REG_LOCAL_GPIO_DATA_GPIO_OUT_SRC(offset) : 0;
+	}
+
+	ret = regmap_update_bits(priv->regmap, UB953_REG_LOCAL_GPIO_DATA,
+				 mask, val);
 	if (ret)
 		return ret;
 
@@ -330,10 +349,30 @@ static int ub953_gpio_of_xlate(struct gpio_chip *gc,
 			       const struct of_phandle_args *gpiospec,
 			       u32 *flags)
 {
+	struct ub953_data *priv = gpiochip_get_data(gc);
+	struct device *dev = &priv->client->dev;
+	u32 pin;
+
+	if (WARN_ON(gc->of_gpio_n_cells < 2))
+		return -EINVAL;
+
+	if (WARN_ON(gpiospec->args_count < gc->of_gpio_n_cells))
+		return -EINVAL;
+
+	pin = gpiospec->args[0];
+	if (pin >= UB953_NUM_GPIOS) {
+		dev_err(dev, "Invalid GPIO pin number: %u\n", pin);
+		return -EINVAL;
+	}
+
+	/* Store GPIO flags for each pin */
+	priv->gpio_flags[pin] = gpiospec->args[1];
+
+	/* Return standard flags to GPIO core */
 	if (flags)
-		*flags = gpiospec->args[1];
+		*flags = gpiospec->args[1] & ~GPIO_DATA_SOURCE_REMOTE;
 
-	return gpiospec->args[0];
+	return pin;
 }
 
 static int ub953_gpiochip_probe(struct ub953_data *priv)

-- 
2.34.1

[PATCH v6 3/4] media: i2c: ds90ub953: use devm_mutex_init() to simplify code

[Guoniu Zhou]

From: Guoniu Zhou <guoniu.zhou@nxp.com>

Use devm_mutex_init() to simplify the code. No functional change.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
---
Changes in v6:
- Added Reviewed-by tag from Tomi Valkeinen

Changes in v2:
- Move PTR_ERR() in dev_err_probe();
---
 drivers/media/i2c/ds90ub953.c | 33 +++++++++++++--------------------
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/drivers/media/i2c/ds90ub953.c b/drivers/media/i2c/ds90ub953.c
index da63dcfbbbc3..c37d9cbe88b9 100644
--- a/drivers/media/i2c/ds90ub953.c
+++ b/drivers/media/i2c/ds90ub953.c
@@ -1299,7 +1299,9 @@ static int ub953_probe(struct i2c_client *client)
 	if (!priv->plat_data)
 		return dev_err_probe(dev, -ENODEV, "Platform data missing\n");
 
-	mutex_init(&priv->reg_lock);
+	ret = devm_mutex_init(dev, &priv->reg_lock);
+	if (ret)
+		return ret;
 
 	/*
 	 * Initialize to invalid values so that the first reg writes will
@@ -1308,32 +1310,26 @@ static int ub953_probe(struct i2c_client *client)
 	priv->current_indirect_target = 0xff;
 
 	priv->regmap = devm_regmap_init_i2c(client, &ub953_regmap_config);
-	if (IS_ERR(priv->regmap)) {
-		ret = PTR_ERR(priv->regmap);
-		dev_err_probe(dev, ret, "Failed to init regmap\n");
-		goto err_mutex_destroy;
-	}
+	if (IS_ERR(priv->regmap))
+		return dev_err_probe(dev, PTR_ERR(priv->regmap),
+				     "Failed to init regmap\n");
 
 	priv->clkin = devm_clk_get_optional(dev, "clkin");
-	if (IS_ERR(priv->clkin)) {
-		ret = PTR_ERR(priv->clkin);
-		dev_err_probe(dev, ret, "failed to parse 'clkin'\n");
-		goto err_mutex_destroy;
-	}
+	if (IS_ERR(priv->clkin))
+		return dev_err_probe(dev, PTR_ERR(priv->clkin),
+				     "Failed to parse 'clkin'\n");
 
 	ret = ub953_parse_dt(priv);
 	if (ret)
-		goto err_mutex_destroy;
+		return ret;
 
 	ret = ub953_hw_init(priv);
 	if (ret)
-		goto err_mutex_destroy;
+		return ret;
 
 	ret = ub953_gpiochip_probe(priv);
-	if (ret) {
-		dev_err_probe(dev, ret, "Failed to init gpiochip\n");
-		goto err_mutex_destroy;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to init gpiochip\n");
 
 	ret = ub953_register_clkout(priv);
 	if (ret) {
@@ -1357,8 +1353,6 @@ static int ub953_probe(struct i2c_client *client)
 	ub953_subdev_uninit(priv);
 err_gpiochip_remove:
 	ub953_gpiochip_remove(priv);
-err_mutex_destroy:
-	mutex_destroy(&priv->reg_lock);
 
 	return ret;
 }
@@ -1373,7 +1367,6 @@ static void ub953_remove(struct i2c_client *client)
 	ub953_subdev_uninit(priv);
 
 	ub953_gpiochip_remove(priv);
-	mutex_destroy(&priv->reg_lock);
 }
 
 static const struct ub953_hw_data ds90ub953_hw = {

-- 
2.34.1

[PATCH v6 4/4] media: i2c: ds90ub953: use guard() to simplify code

[Guoniu Zhou]

From: Guoniu Zhou <guoniu.zhou@nxp.com>

Use guard() to simplify mutex locking. No functional change.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
---
Changes in v6:
- Replace label "err" with "out"
---
 drivers/media/i2c/ds90ub953.c | 34 +++++++++++++---------------------
 1 file changed, 13 insertions(+), 21 deletions(-)

diff --git a/drivers/media/i2c/ds90ub953.c b/drivers/media/i2c/ds90ub953.c
index c37d9cbe88b9..a351d366cc3e 100644
--- a/drivers/media/i2c/ds90ub953.c
+++ b/drivers/media/i2c/ds90ub953.c
@@ -114,20 +114,18 @@ static int ub953_read(struct ub953_data *priv, u8 reg, u8 *val, int *err)
 	if (err && *err)
 		return *err;
 
-	mutex_lock(&priv->reg_lock);
+	guard(mutex)(&priv->reg_lock);
 
 	ret = regmap_read(priv->regmap, reg, &v);
 	if (ret) {
 		dev_err(&priv->client->dev, "Cannot read register 0x%02x: %d\n",
 			reg, ret);
-		goto out_unlock;
+		goto out;
 	}
 
 	*val = v;
 
-out_unlock:
-	mutex_unlock(&priv->reg_lock);
-
+out:
 	if (ret && err)
 		*err = ret;
 
@@ -141,15 +139,13 @@ static int ub953_write(struct ub953_data *priv, u8 reg, u8 val, int *err)
 	if (err && *err)
 		return *err;
 
-	mutex_lock(&priv->reg_lock);
+	guard(mutex)(&priv->reg_lock);
 
 	ret = regmap_write(priv->regmap, reg, val);
 	if (ret)
 		dev_err(&priv->client->dev,
 			"Cannot write register 0x%02x: %d\n", reg, ret);
 
-	mutex_unlock(&priv->reg_lock);
-
 	if (ret && err)
 		*err = ret;
 
@@ -186,18 +182,18 @@ static int ub953_read_ind(struct ub953_data *priv, u8 block, u8 reg, u8 *val,
 	if (err && *err)
 		return *err;
 
-	mutex_lock(&priv->reg_lock);
+	guard(mutex)(&priv->reg_lock);
 
 	ret = ub953_select_ind_reg_block(priv, block);
 	if (ret)
-		goto out_unlock;
+		goto out;
 
 	ret = regmap_write(priv->regmap, UB953_REG_IND_ACC_ADDR, reg);
 	if (ret) {
 		dev_err(&priv->client->dev,
 			"Write to IND_ACC_ADDR failed when reading %u:0x%02x: %d\n",
 			block, reg, ret);
-		goto out_unlock;
+		goto out;
 	}
 
 	ret = regmap_read(priv->regmap, UB953_REG_IND_ACC_DATA, &v);
@@ -205,14 +201,12 @@ static int ub953_read_ind(struct ub953_data *priv, u8 block, u8 reg, u8 *val,
 		dev_err(&priv->client->dev,
 			"Write to IND_ACC_DATA failed when reading %u:0x%02x: %d\n",
 			block, reg, ret);
-		goto out_unlock;
+		goto out;
 	}
 
 	*val = v;
 
-out_unlock:
-	mutex_unlock(&priv->reg_lock);
-
+out:
 	if (ret && err)
 		*err = ret;
 
@@ -228,18 +222,18 @@ static int ub953_write_ind(struct ub953_data *priv, u8 block, u8 reg, u8 val,
 	if (err && *err)
 		return *err;
 
-	mutex_lock(&priv->reg_lock);
+	guard(mutex)(&priv->reg_lock);
 
 	ret = ub953_select_ind_reg_block(priv, block);
 	if (ret)
-		goto out_unlock;
+		goto out;
 
 	ret = regmap_write(priv->regmap, UB953_REG_IND_ACC_ADDR, reg);
 	if (ret) {
 		dev_err(&priv->client->dev,
 			"Write to IND_ACC_ADDR failed when writing %u:0x%02x: %d\n",
 			block, reg, ret);
-		goto out_unlock;
+		goto out;
 	}
 
 	ret = regmap_write(priv->regmap, UB953_REG_IND_ACC_DATA, val);
@@ -249,9 +243,7 @@ static int ub953_write_ind(struct ub953_data *priv, u8 block, u8 reg, u8 val,
 			block, reg, ret);
 	}
 
-out_unlock:
-	mutex_unlock(&priv->reg_lock);
-
+out:
 	if (ret && err)
 		*err = ret;
 

-- 
2.34.1

Re: [PATCH v6 3/4] media: i2c: ds90ub953: use devm_mutex_init() to simplify code

[Bartosz Golaszewski]

On Fri, 24 Apr 2026 03:42:26 +0200, Guoniu Zhou <guoniu.zhou@oss.nxp.com> said:
> From: Guoniu Zhou <guoniu.zhou@nxp.com>
>
> Use devm_mutex_init() to simplify the code. No functional change.
>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> ---
> Changes in v6:
> - Added Reviewed-by tag from Tomi Valkeinen
>
> Changes in v2:
> - Move PTR_ERR() in dev_err_probe();
> ---

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

Re: [PATCH v6 4/4] media: i2c: ds90ub953: use guard() to simplify code

[Bartosz Golaszewski]

On Fri, 24 Apr 2026 03:42:27 +0200, Guoniu Zhou <guoniu.zhou@oss.nxp.com> said:
> From: Guoniu Zhou <guoniu.zhou@nxp.com>
>
> Use guard() to simplify mutex locking. No functional change.
>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> ---

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

Re: [PATCH v6 1/4] dt-bindings: media: ti,ds90ub953: Add support for remote GPIO data source

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3050 bytes --]

On Fri, Apr 24, 2026 at 09:42:24AM +0800, Guoniu Zhou wrote:
> From: Guoniu Zhou <guoniu.zhou@nxp.com>
> 
> The DS90UB953 supports four pins, GPIO0 through GPIO3. When enabled as an
> output, it can be programed to output local data or remote data coming
> from the remote compatible deserializer.
> 
> Add GPIO flag in second cell to select remote GPIO data source.
> 
> Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> ---
> Changes in v6:
> - Added GPIO_DATA_SOURCE_REMOTE flag to dt-bindings/gpio/gpio.h
> - Updated dt-bindings documentation accordingly
> - Dropped Reviewed-by tag from Rob Herring due to significant binding change
> 
> Changes in v5:
> - Improve the description for "#gpio-cells" as commented by Conor.
> 
> Changes in v4:
> - Use folder block instead of literal block for #gpio-cell property description.
> 
> Changes in v3:
> - Make GPIO range from 0-3 to 0-7 to support GPIO data from remote
>   compatible deserializer suggested by Rob instead of adding third
>   cell for GPIO controller.
> 
> Changes in v2:
> - Remove new property ti,gpio-data
> - Add third cell for GPIO controller to select GPIO output source.
> ---
>  Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml | 6 ++++--
>  include/dt-bindings/gpio/gpio.h                               | 8 ++++++++
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
> index 2e129bf573b7..da63771bc236 100644
> --- a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
> @@ -21,8 +21,10 @@ properties:
>    '#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, 3].
> +      First cell is the GPIO pin number (0-3) and the second cell is used
> +      to specify flags. See <dt-bindings/gpio/gpio.h> for available flags
> +      including GPIO_DATA_SOURCE_REMOTE for remote GPIO data source.
> +      Flags can be OR'd together.
>  
>    gpio-controller: true
>  
> diff --git a/include/dt-bindings/gpio/gpio.h b/include/dt-bindings/gpio/gpio.h
> index b5d531237448..d04a494d96ad 100644
> --- a/include/dt-bindings/gpio/gpio.h
> +++ b/include/dt-bindings/gpio/gpio.h
> @@ -42,4 +42,12 @@
>  /* Bit 6 express pull disable */
>  #define GPIO_PULL_DISABLE 64
>  
> +/*
> + * Bit 24 indicates the GPIO data source is from a remote device.

Why 24, not 7?

> + * This is used in serializer/deserializer setups where the GPIO pin
> + * on the local device (e.g., TI DS90UB953 serializer) reflects the
> + * state of a GPIO on the remote device (e.g., TI DS90UB960 deserializer).
> + */
> +#define GPIO_DATA_SOURCE_REMOTE		0x01000000

And why the divergent formatting compared to other defines in this file?

> +
>  #endif
> 
> -- 
> 2.34.1
> 

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

Re: [PATCH v6 1/4] dt-bindings: media: ti,ds90ub953: Add support for remote GPIO data source

[Linus Walleij]

Hi Guoniu,

thanks for your patch!

n Fri, Apr 24, 2026 at 3:40 AM Guoniu Zhou <guoniu.zhou@oss.nxp.com> wrote:

> The DS90UB953 supports four pins, GPIO0 through GPIO3. When enabled as an
> output, it can be programed to output local data or remote data coming
> from the remote compatible deserializer.
>
> Add GPIO flag in second cell to select remote GPIO data source.
>
> Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
(...)
> ---
> Changes in v6:
> - Added GPIO_DATA_SOURCE_REMOTE flag to dt-bindings/gpio/gpio.h

(...)
> +/*
> + * Bit 24 indicates the GPIO data source is from a remote device.
> + * This is used in serializer/deserializer setups where the GPIO pin
> + * on the local device (e.g., TI DS90UB953 serializer) reflects the
> + * state of a GPIO on the remote device (e.g., TI DS90UB960 deserializer).
> + */
> +#define GPIO_DATA_SOURCE_REMOTE                0x01000000

So this is added to all GPIO controllers ever in existance as something
fundamental about GPIO that may need its own flag.

I doubt this.

I understand that the driver needs to deal with this in a per-gpio-line
basis though, have you considered:

1. Just hard-coding this into the driver based on the .compatible
  string, if the remote GPIOs are always the same for this TI thing?

2. If it is just for one particular system, you *could* actually have
  a table/mask inside the driver for this:
  if (of_machine_is_compatible("my-funky-system")) which will
  kick in only on that very machine,

3. If you really want to store the information in the media i2c
  device node, add some custom property like this:
  ti,remote-sources = <0x0000001f>;
  where a bit is set to 1 for each GPIO which is remote.

Putting flags on the GPIO lines themselves seems too complex
and system-specific.

Yours,
Linus Walleij

RE: [PATCH v6 1/4] dt-bindings: media: ti,ds90ub953: Add support for remote GPIO data source

[G.N. Zhou (OSS)]

Hi Conor,

Thanks for your review.

> -----Original Message-----
> From: Conor Dooley <conor@kernel.org>
> Sent: Saturday, April 25, 2026 1:09 AM
> To: G.N. Zhou (OSS) <guoniu.zhou@oss.nxp.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>; Mauro Carvalho
> Chehab <mchehab@kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof
> Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Frank
> Li <frank.li@nxp.com>; Vladimir Zapolskiy <vz@mleia.com>; Linus Walleij
> <linusw@kernel.org>; Bartosz Golaszewski <brgl@kernel.org>; linux-
> media@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; imx@lists.linux.dev; linux-gpio@vger.kernel.org; G.N.
> Zhou <guoniu.zhou@nxp.com>
> Subject: Re: [PATCH v6 1/4] dt-bindings: media: ti,ds90ub953: Add support for
> remote GPIO data source
> 
> On Fri, Apr 24, 2026 at 09:42:24AM +0800, Guoniu Zhou wrote:
> > From: Guoniu Zhou <guoniu.zhou@nxp.com>
> >
> > The DS90UB953 supports four pins, GPIO0 through GPIO3. When enabled as
> > an output, it can be programed to output local data or remote data
> > coming from the remote compatible deserializer.
> >
> > Add GPIO flag in second cell to select remote GPIO data source.
> >
> > Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> > ---
> > Changes in v6:
> > - Added GPIO_DATA_SOURCE_REMOTE flag to dt-bindings/gpio/gpio.h
> > - Updated dt-bindings documentation accordingly
> > - Dropped Reviewed-by tag from Rob Herring due to significant binding
> > change
> >
> > Changes in v5:
> > - Improve the description for "#gpio-cells" as commented by Conor.
> >
> > Changes in v4:
> > - Use folder block instead of literal block for #gpio-cell property description.
> >
> > Changes in v3:
> > - Make GPIO range from 0-3 to 0-7 to support GPIO data from remote
> >   compatible deserializer suggested by Rob instead of adding third
> >   cell for GPIO controller.
> >
> > Changes in v2:
> > - Remove new property ti,gpio-data
> > - Add third cell for GPIO controller to select GPIO output source.
> > ---
> >  Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml | 6 ++++-
> -
> >  include/dt-bindings/gpio/gpio.h                               | 8 ++++++++
> >  2 files changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
> > b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
> > index 2e129bf573b7..da63771bc236 100644
> > --- a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
> > +++ b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
> > @@ -21,8 +21,10 @@ properties:
> >    '#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, 3].
> > +      First cell is the GPIO pin number (0-3) and the second cell is used
> > +      to specify flags. See <dt-bindings/gpio/gpio.h> for available flags
> > +      including GPIO_DATA_SOURCE_REMOTE for remote GPIO data source.
> > +      Flags can be OR'd together.
> >
> >    gpio-controller: true
> >
> > diff --git a/include/dt-bindings/gpio/gpio.h
> > b/include/dt-bindings/gpio/gpio.h index b5d531237448..d04a494d96ad
> > 100644
> > --- a/include/dt-bindings/gpio/gpio.h
> > +++ b/include/dt-bindings/gpio/gpio.h
> > @@ -42,4 +42,12 @@
> >  /* Bit 6 express pull disable */
> >  #define GPIO_PULL_DISABLE 64
> >
> > +/*
> > + * Bit 24 indicates the GPIO data source is from a remote device.
> 
> Why 24, not 7?

Good question. I chose bit 24 for the following reasons:
- There is no clear guidance in the documentation on how to add custom
   GPIO flags, so I wanted to ensure sufficient separation from the generic
   GPIO flags to avoid potential conflicts.
- The generic GPIO flags currently use the lower bits (0-6). By using
   bit 24, I'm leaving ample room (bits 7-23) for future expansion of
   generic flags without risking collision.
- This also reserves space (bits 25-31) for potential future custom
   flags that may be needed by this or other drivers.

However, if there's a preferred convention or range for driver-specific
GPIO flags that I'm not aware of, I'm happy to adjust this.

> 
> > + * This is used in serializer/deserializer setups where the GPIO pin
> > + * on the local device (e.g., TI DS90UB953 serializer) reflects the
> > + * state of a GPIO on the remote device (e.g., TI DS90UB960 deserializer).
> > + */
> > +#define GPIO_DATA_SOURCE_REMOTE		0x01000000
> 
> And why the divergent formatting compared to other defines in this file?

You're right about the formatting inconsistency.

The existing decimal notation (8, 16, 32, 64) works well for lower bits, but
16777216 (decimal for bit 24) is much less readable than 0x01000000. It's
also harder to verify correctness and more error-prone.

If consistency with the existing style is preferred, I can use decimal, but
I'd suggest considering updating all these defines to use BIT() hex for better
Readability and maintainability. What's your preference?

> 
> > +
> >  #endif
> >
> > --
> > 2.34.1
> >

RE: [PATCH v6 1/4] dt-bindings: media: ti,ds90ub953: Add support for remote GPIO data source

[G.N. Zhou (OSS)]

Hi Linus,

Thanks for your review.

> -----Original Message-----
> From: Linus Walleij <linusw@kernel.org>
> Sent: Sunday, April 26, 2026 4:36 PM
> To: G.N. Zhou (OSS) <guoniu.zhou@oss.nxp.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>; Mauro Carvalho
> Chehab <mchehab@kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof
> Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Frank
> Li <frank.li@nxp.com>; Vladimir Zapolskiy <vz@mleia.com>; Bartosz
> Golaszewski <brgl@kernel.org>; linux-media@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; imx@lists.linux.dev;
> linux-gpio@vger.kernel.org; G.N. Zhou <guoniu.zhou@nxp.com>
> Subject: Re: [PATCH v6 1/4] dt-bindings: media: ti,ds90ub953: Add support for
> remote GPIO data source
> 
> Hi Guoniu,
> 
> thanks for your patch!
> 
> n Fri, Apr 24, 2026 at 3:40 AM Guoniu Zhou <guoniu.zhou@oss.nxp.com>
> wrote:
> 
> > The DS90UB953 supports four pins, GPIO0 through GPIO3. When enabled as
> > an output, it can be programed to output local data or remote data
> > coming from the remote compatible deserializer.
> >
> > Add GPIO flag in second cell to select remote GPIO data source.
> >
> > Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> (...)
> > ---
> > Changes in v6:
> > - Added GPIO_DATA_SOURCE_REMOTE flag to dt-bindings/gpio/gpio.h
> 
> (...)
> > +/*
> > + * Bit 24 indicates the GPIO data source is from a remote device.
> > + * This is used in serializer/deserializer setups where the GPIO pin
> > + * on the local device (e.g., TI DS90UB953 serializer) reflects the
> > + * state of a GPIO on the remote device (e.g., TI DS90UB960 deserializer).
> > + */
> > +#define GPIO_DATA_SOURCE_REMOTE                0x01000000
> 
> So this is added to all GPIO controllers ever in existance as something
> fundamental about GPIO that may need its own flag.

You're absolutely right that adding this to the global GPIO flags is too
broad for what is essentially a device-specific requirement.

> 
> I doubt this.
> 
> I understand that the driver needs to deal with this in a per-gpio-line basis
> though, have you considered:
> 
> 1. Just hard-coding this into the driver based on the .compatible
>   string, if the remote GPIOs are always the same for this TI thing?
> 
> 2. If it is just for one particular system, you *could* actually have
>   a table/mask inside the driver for this:
>   if (of_machine_is_compatible("my-funky-system")) which will
>   kick in only on that very machine,
> 
> 3. If you really want to store the information in the media i2c
>   device node, add some custom property like this:
>   ti,remote-sources = <0x0000001f>;
>   where a bit is set to 1 for each GPIO which is remote.
> 
> Putting flags on the GPIO lines themselves seems too complex and system-
> specific.

Thank you for the detailed feedback.

After considering your suggestions, I think option 3 (custom device property)
is the most appropriate approach for this case.

However, I initially implemented this using a custom device property
(ti,gpio-data) in v1 [1], and Vladimir rejected that approach.

In v2, I extended the GPIO cell to use a third cell to mark remote data,
but unfortunately, this broke the ABI. Rob suggested using a flag in the
second cell or extending the GPIO range to represent remote data [2].

I adopted the extended GPIO range approach in v3, but in v5, Tomi
suggested using a custom GPIO flag instead [3].

I attempted to implement this, but couldn't find guidance on how to add
custom GPIO flags, so I added it to include/dt-bindings/gpio/gpio.h and
choose bit 24, see the reasons in [4].

Given this history, what would be the acceptable solution? Could the
maintainers align on the preferred approach. I'm happy to implement
whichever solution is acceptable, but I need clear direction to avoid going
in circles.

[1]: https://lore.kernel.org/all/e526e93e-1373-4f1e-82ab-6b22d68211cd@mleia.com/
[2]: https://lore.kernel.org/all/20250915015309.GA2329507-robh@kernel.org/
[3]: https://lore.kernel.org/all/d63bff5a-1a37-4162-8b75-bf62787a0782@ideasonboard.com/
[4]: https://lore.kernel.org/all/AS8PR04MB9080CEB000F4F1C9043E8C21FA362@AS8PR04MB9080.eurprd04.prod.outlook.com/#t
> 
> Yours,
> Linus Walleij

Re: [PATCH v6 1/4] dt-bindings: media: ti,ds90ub953: Add support for remote GPIO data source

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 5620 bytes --]

On Mon, Apr 27, 2026 at 08:12:10AM +0000, G.N. Zhou (OSS) wrote:
> Hi Conor,
> 
> Thanks for your review.
> 
> > -----Original Message-----
> > From: Conor Dooley <conor@kernel.org>
> > Sent: Saturday, April 25, 2026 1:09 AM
> > To: G.N. Zhou (OSS) <guoniu.zhou@oss.nxp.com>
> > Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>; Mauro Carvalho
> > Chehab <mchehab@kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof
> > Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Frank
> > Li <frank.li@nxp.com>; Vladimir Zapolskiy <vz@mleia.com>; Linus Walleij
> > <linusw@kernel.org>; Bartosz Golaszewski <brgl@kernel.org>; linux-
> > media@vger.kernel.org; devicetree@vger.kernel.org; linux-
> > kernel@vger.kernel.org; imx@lists.linux.dev; linux-gpio@vger.kernel.org; G.N.
> > Zhou <guoniu.zhou@nxp.com>
> > Subject: Re: [PATCH v6 1/4] dt-bindings: media: ti,ds90ub953: Add support for
> > remote GPIO data source
> > 
> > On Fri, Apr 24, 2026 at 09:42:24AM +0800, Guoniu Zhou wrote:
> > > From: Guoniu Zhou <guoniu.zhou@nxp.com>
> > >
> > > The DS90UB953 supports four pins, GPIO0 through GPIO3. When enabled as
> > > an output, it can be programed to output local data or remote data
> > > coming from the remote compatible deserializer.
> > >
> > > Add GPIO flag in second cell to select remote GPIO data source.
> > >
> > > Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> > > ---
> > > Changes in v6:
> > > - Added GPIO_DATA_SOURCE_REMOTE flag to dt-bindings/gpio/gpio.h
> > > - Updated dt-bindings documentation accordingly
> > > - Dropped Reviewed-by tag from Rob Herring due to significant binding
> > > change
> > >
> > > Changes in v5:
> > > - Improve the description for "#gpio-cells" as commented by Conor.
> > >
> > > Changes in v4:
> > > - Use folder block instead of literal block for #gpio-cell property description.
> > >
> > > Changes in v3:
> > > - Make GPIO range from 0-3 to 0-7 to support GPIO data from remote
> > >   compatible deserializer suggested by Rob instead of adding third
> > >   cell for GPIO controller.
> > >
> > > Changes in v2:
> > > - Remove new property ti,gpio-data
> > > - Add third cell for GPIO controller to select GPIO output source.
> > > ---
> > >  Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml | 6 ++++-
> > -
> > >  include/dt-bindings/gpio/gpio.h                               | 8 ++++++++
> > >  2 files changed, 12 insertions(+), 2 deletions(-)
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
> > > b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
> > > index 2e129bf573b7..da63771bc236 100644
> > > --- a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
> > > +++ b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
> > > @@ -21,8 +21,10 @@ properties:
> > >    '#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, 3].
> > > +      First cell is the GPIO pin number (0-3) and the second cell is used
> > > +      to specify flags. See <dt-bindings/gpio/gpio.h> for available flags
> > > +      including GPIO_DATA_SOURCE_REMOTE for remote GPIO data source.
> > > +      Flags can be OR'd together.
> > >
> > >    gpio-controller: true
> > >
> > > diff --git a/include/dt-bindings/gpio/gpio.h
> > > b/include/dt-bindings/gpio/gpio.h index b5d531237448..d04a494d96ad
> > > 100644
> > > --- a/include/dt-bindings/gpio/gpio.h
> > > +++ b/include/dt-bindings/gpio/gpio.h
> > > @@ -42,4 +42,12 @@
> > >  /* Bit 6 express pull disable */
> > >  #define GPIO_PULL_DISABLE 64
> > >
> > > +/*
> > > + * Bit 24 indicates the GPIO data source is from a remote device.
> > 
> > Why 24, not 7?
> 
> Good question. I chose bit 24 for the following reasons:
> - There is no clear guidance in the documentation on how to add custom
>    GPIO flags, so I wanted to ensure sufficient separation from the generic
>    GPIO flags to avoid potential conflicts.
> - The generic GPIO flags currently use the lower bits (0-6). By using
>    bit 24, I'm leaving ample room (bits 7-23) for future expansion of
>    generic flags without risking collision.
> - This also reserves space (bits 25-31) for potential future custom
>    flags that may be needed by this or other drivers.
> 
> However, if there's a preferred convention or range for driver-specific
> GPIO flags that I'm not aware of, I'm happy to adjust this.
> 
> > 
> > > + * This is used in serializer/deserializer setups where the GPIO pin
> > > + * on the local device (e.g., TI DS90UB953 serializer) reflects the
> > > + * state of a GPIO on the remote device (e.g., TI DS90UB960 deserializer).
> > > + */
> > > +#define GPIO_DATA_SOURCE_REMOTE		0x01000000
> > 
> > And why the divergent formatting compared to other defines in this file?
> 
> You're right about the formatting inconsistency.
> 
> The existing decimal notation (8, 16, 32, 64) works well for lower bits, but
> 16777216 (decimal for bit 24) is much less readable than 0x01000000. It's
> also harder to verify correctness and more error-prone.
> 
> If consistency with the existing style is preferred, I can use decimal, but
> I'd suggest considering updating all these defines to use BIT() hex for better
> Readability and maintainability. What's your preference?

FWIW, I'm not ignoring you, but I won't reply until the more important
discussion with Linus has been resolved.

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

Re: [PATCH v6 1/4] dt-bindings: media: ti,ds90ub953: Add support for remote GPIO data source

[Linus Walleij]

On Mon, Apr 27, 2026 at 11:13 AM G.N. Zhou (OSS)
<guoniu.zhou@oss.nxp.com> wrote:
> [Me]

> > I understand that the driver needs to deal with this in a per-gpio-line basis
> > though, have you considered:
> >
> > 1. Just hard-coding this into the driver based on the .compatible
> >   string, if the remote GPIOs are always the same for this TI thing?
> >
> > 2. If it is just for one particular system, you *could* actually have
> >   a table/mask inside the driver for this:
> >   if (of_machine_is_compatible("my-funky-system")) which will
> >   kick in only on that very machine,
> >
> > 3. If you really want to store the information in the media i2c
> >   device node, add some custom property like this:
> >   ti,remote-sources = <0x0000001f>;
> >   where a bit is set to 1 for each GPIO which is remote.
> >
> > Putting flags on the GPIO lines themselves seems too complex and system-
> > specific.
>
> Thank you for the detailed feedback.
>
> After considering your suggestions, I think option 3 (custom device property)
> is the most appropriate approach for this case.

Why is that the most appropriate?

I think (1) is most appropriate, if the hardware with this compatible
always looks like this. You need to answer the question if this
is a per-system flag for the GPIO lines or something that is *always*
applicable for a device with compatible ti,ds90ub9NN-q1?

If it for example always applies to ti,ds90ub971-q1, then make that
compatible decide how to handle indvidual line, just write
code for it. That is case (1).

If this setting depends on how the serializer is integrated and the
remote setting may apply to some systems with this device and not others,
you have options (2) and (3).

> However, I initially implemented this using a custom device property
> (ti,gpio-data) in v1 [1], and Vladimir rejected that approach.

Vladimir is saying that the driver code should handle this
without any extra DT properties. That can be done with approach
(1) and (2). But I don't know about that.

I think the basic problem with the patch is that no-one (myself included)
apart from you understand what a remote serializer is, why it is
remote and what that means, how the mechanism between the
components making up this essentially works etc. I.e. a much
longer and more detailed commit message and binding explaining
very cleary what this is and how it works and why the special
property is needed on some lines, and how it is a property
of some specific way of integrating this GPIO controller.

If a custom property should be used ti,gpio-data is too generic,
come up with a property name that actually says what it is all
about and which anyone would understand. "gpio-data" is a
bit "the thing that does the thing" and overly generic term.

Yours,
Linus Walleij

RE: [PATCH v6 1/4] dt-bindings: media: ti,ds90ub953: Add support for remote GPIO data source

[G.N. Zhou (OSS)]

Hi Linus,

> -----Original Message-----
> From: Linus Walleij <linusw@kernel.org>
> Sent: Tuesday, April 28, 2026 4:50 AM
> To: G.N. Zhou (OSS) <guoniu.zhou@oss.nxp.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>; Mauro Carvalho
> Chehab <mchehab@kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof
> Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Frank
> Li <frank.li@nxp.com>; Vladimir Zapolskiy <vz@mleia.com>; Bartosz
> Golaszewski <brgl@kernel.org>; linux-media@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; imx@lists.linux.dev;
> linux-gpio@vger.kernel.org; G.N. Zhou <guoniu.zhou@nxp.com>
> Subject: Re: [PATCH v6 1/4] dt-bindings: media: ti,ds90ub953: Add support for
> remote GPIO data source
> 
> On Mon, Apr 27, 2026 at 11:13 AM G.N. Zhou (OSS)
> <guoniu.zhou@oss.nxp.com> wrote:
> > [Me]
> 
> > > I understand that the driver needs to deal with this in a
> > > per-gpio-line basis though, have you considered:
> > >
> > > 1. Just hard-coding this into the driver based on the .compatible
> > >   string, if the remote GPIOs are always the same for this TI thing?
> > >
> > > 2. If it is just for one particular system, you *could* actually have
> > >   a table/mask inside the driver for this:
> > >   if (of_machine_is_compatible("my-funky-system")) which will
> > >   kick in only on that very machine,
> > >
> > > 3. If you really want to store the information in the media i2c
> > >   device node, add some custom property like this:
> > >   ti,remote-sources = <0x0000001f>;
> > >   where a bit is set to 1 for each GPIO which is remote.
> > >
> > > Putting flags on the GPIO lines themselves seems too complex and
> > > system- specific.
> >
> > Thank you for the detailed feedback.
> >
> > After considering your suggestions, I think option 3 (custom device
> > property) is the most appropriate approach for this case.
> 
> Why is that the most appropriate?
> 
> I think (1) is most appropriate, if the hardware with this compatible always
> looks like this. You need to answer the question if this is a per-system flag for
> the GPIO lines or something that is *always* applicable for a device with
> compatible ti,ds90ub9NN-q1?
> 
> If it for example always applies to ti,ds90ub971-q1, then make that compatible
> decide how to handle indvidual line, just write code for it. That is case (1).
> 
> If this setting depends on how the serializer is integrated and the remote
> setting may apply to some systems with this device and not others, you have
> options (2) and (3).
> 
> > However, I initially implemented this using a custom device property
> > (ti,gpio-data) in v1 [1], and Vladimir rejected that approach.
> 
> Vladimir is saying that the driver code should handle this without any extra DT
> properties. That can be done with approach
> (1) and (2). But I don't know about that.
> 
> I think the basic problem with the patch is that no-one (myself included) apart
> from you understand what a remote serializer is, why it is remote and what
> that means, how the mechanism between the components making up this
> essentially works etc. I.e. a much longer and more detailed commit message
> and binding explaining very cleary what this is and how it works and why the
> special property is needed on some lines, and how it is a property of some
> specific way of integrating this GPIO controller.
> 
> If a custom property should be used ti,gpio-data is too generic, come up with a
> property name that actually says what it is all about and which anyone would
> understand. "gpio-data" is a bit "the thing that does the thing" and overly
> generic term.

Thank you for this feedback. Let me explain the hardware architecture and the
reason I chose option (3).

Hardware Architecture:
SoC --- I2C --- DS90UB960 (Deserializer) ---+--- FPD-Link --- DS90UB953 (Ser) --- OX03C10 Camera
                                                                           +--- FPD-Link --- DS90UB953 (Ser) --- OX03C10 Camera
                                                                           +--- FPD-Link --- DS90UB953 (Ser) --- OX03C10 Camera
                                                                           +--- FPD-Link --- DS90UB953 (Ser) --- OX03C10 Camera

Each DS90UB953 serializer has 4 GPIO pins. One of these GPIOs connects to the
OX03C10 camera's FSIN (frame sync input) pin for multi-camera synchronization.

The Problem - Remote vs Local GPIO Data Source:
-----------------------------------------------
The DS90UB953's GPIO pins can operate in two modes:
1. LOCAL mode: GPIO data comes from the I2C-controlled GPIO register
   - Standard GPIO expander behavior
   - Data written via I2C controls the pin state

2. REMOTE mode: GPIO data comes from the DS90UB960 deserializer
   - The DS90UB960 generates a frame sync signal internally
   - This signal is transmitted to the DS90UB953 via the FPD-Link back channel
   - The DS90UB953 routes this remote signal to the specified GPIO pin
   - The GPIO pin state is NOT controlled by I2C register writes

For camera synchronization, we need REMOTE mode: the FSIN GPIO must receive
the frame sync signal from the deserializer, not from local I2C writes.

Regarding option1, the remote GPIO configuration can vary between different board
designs using the same TI device, so hard-coding based on .compatible wouldn't be
flexible enough. 

Regarding option 2, if I understand correctly, driver would require adding a new
compatible string or mask entry for every different board configuration, which
doesn't scale well.

> 
> Yours,
> Linus Walleij

Re: [PATCH v6 1/4] dt-bindings: media: ti,ds90ub953: Add support for remote GPIO data source

[Tomi Valkeinen]

Hi,

On 28/04/2026 09:17, G.N. Zhou (OSS) wrote:
> Hi Linus,
> 
>> -----Original Message-----
>> From: Linus Walleij <linusw@kernel.org>
>> Sent: Tuesday, April 28, 2026 4:50 AM
>> To: G.N. Zhou (OSS) <guoniu.zhou@oss.nxp.com>
>> Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>; Mauro Carvalho
>> Chehab <mchehab@kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof
>> Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Frank
>> Li <frank.li@nxp.com>; Vladimir Zapolskiy <vz@mleia.com>; Bartosz
>> Golaszewski <brgl@kernel.org>; linux-media@vger.kernel.org;
>> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; imx@lists.linux.dev;
>> linux-gpio@vger.kernel.org; G.N. Zhou <guoniu.zhou@nxp.com>
>> Subject: Re: [PATCH v6 1/4] dt-bindings: media: ti,ds90ub953: Add support for
>> remote GPIO data source
>>
>> On Mon, Apr 27, 2026 at 11:13 AM G.N. Zhou (OSS)
>> <guoniu.zhou@oss.nxp.com> wrote:
>>> [Me]
>>
>>>> I understand that the driver needs to deal with this in a
>>>> per-gpio-line basis though, have you considered:
>>>>
>>>> 1. Just hard-coding this into the driver based on the .compatible
>>>>    string, if the remote GPIOs are always the same for this TI thing?
>>>>
>>>> 2. If it is just for one particular system, you *could* actually have
>>>>    a table/mask inside the driver for this:
>>>>    if (of_machine_is_compatible("my-funky-system")) which will
>>>>    kick in only on that very machine,
>>>>
>>>> 3. If you really want to store the information in the media i2c
>>>>    device node, add some custom property like this:
>>>>    ti,remote-sources = <0x0000001f>;
>>>>    where a bit is set to 1 for each GPIO which is remote.
>>>>
>>>> Putting flags on the GPIO lines themselves seems too complex and
>>>> system- specific.
>>>
>>> Thank you for the detailed feedback.
>>>
>>> After considering your suggestions, I think option 3 (custom device
>>> property) is the most appropriate approach for this case.
>>
>> Why is that the most appropriate?
>>
>> I think (1) is most appropriate, if the hardware with this compatible always
>> looks like this. You need to answer the question if this is a per-system flag for
>> the GPIO lines or something that is *always* applicable for a device with
>> compatible ti,ds90ub9NN-q1?
>>
>> If it for example always applies to ti,ds90ub971-q1, then make that compatible
>> decide how to handle indvidual line, just write code for it. That is case (1).
>>
>> If this setting depends on how the serializer is integrated and the remote
>> setting may apply to some systems with this device and not others, you have
>> options (2) and (3).
>>
>>> However, I initially implemented this using a custom device property
>>> (ti,gpio-data) in v1 [1], and Vladimir rejected that approach.
>>
>> Vladimir is saying that the driver code should handle this without any extra DT
>> properties. That can be done with approach
>> (1) and (2). But I don't know about that.
>>
>> I think the basic problem with the patch is that no-one (myself included) apart
>> from you understand what a remote serializer is, why it is remote and what
>> that means, how the mechanism between the components making up this
>> essentially works etc. I.e. a much longer and more detailed commit message
>> and binding explaining very cleary what this is and how it works and why the
>> special property is needed on some lines, and how it is a property of some
>> specific way of integrating this GPIO controller.
>>
>> If a custom property should be used ti,gpio-data is too generic, come up with a
>> property name that actually says what it is all about and which anyone would
>> understand. "gpio-data" is a bit "the thing that does the thing" and overly
>> generic term.
> 
> Thank you for this feedback. Let me explain the hardware architecture and the
> reason I chose option (3).
> 
> Hardware Architecture:
> SoC --- I2C --- DS90UB960 (Deserializer) ---+--- FPD-Link --- DS90UB953 (Ser) --- OX03C10 Camera
>                                                                             +--- FPD-Link --- DS90UB953 (Ser) --- OX03C10 Camera
>                                                                             +--- FPD-Link --- DS90UB953 (Ser) --- OX03C10 Camera
>                                                                             +--- FPD-Link --- DS90UB953 (Ser) --- OX03C10 Camera
> 
> Each DS90UB953 serializer has 4 GPIO pins. One of these GPIOs connects to the
> OX03C10 camera's FSIN (frame sync input) pin for multi-camera synchronization.
> 
> The Problem - Remote vs Local GPIO Data Source:
> -----------------------------------------------
> The DS90UB953's GPIO pins can operate in two modes:
> 1. LOCAL mode: GPIO data comes from the I2C-controlled GPIO register
>     - Standard GPIO expander behavior
>     - Data written via I2C controls the pin state
> 
> 2. REMOTE mode: GPIO data comes from the DS90UB960 deserializer
>     - The DS90UB960 generates a frame sync signal internally

While not exactly part of this series, how do you handle UB960 
configuration? I don't remember all the options, but if I'm not 
mistaken, a GPIO can be generated internally (e.g. frame sync you 
mention), but it can also come into UB960 via a pin (could also be used 
for a frame sync). And there were some debug options that can be exposed 
via the GPIOs.

I'm not asking to implement all those, but I think it's worth at least 
going through them, to have an idea if whatever is done now could 
possibly support the other features.

>     - This signal is transmitted to the DS90UB953 via the FPD-Link back channel
>     - The DS90UB953 routes this remote signal to the specified GPIO pin
>     - The GPIO pin state is NOT controlled by I2C register writes
> 
> For camera synchronization, we need REMOTE mode: the FSIN GPIO must receive
> the frame sync signal from the deserializer, not from local I2C writes.
> 
> Regarding option1, the remote GPIO configuration can vary between different board
> designs using the same TI device, so hard-coding based on .compatible wouldn't be
> flexible enough.
> 
> Regarding option 2, if I understand correctly, driver would require adding a new
> compatible string or mask entry for every different board configuration, which
> doesn't scale well.
I think option 3 is okay.

Although I (perhaps naively) think that the approach in this patch feels 
nicest: the user just defines the GPIO the normal way in the dts, and 
marks it with a REMOTE flag. But I think the REMOTE flag should be a 
device specific flag. Are those allowed?

But now, after writing the above, I wonder... If we use the REMOTE flag, 
that will be in the consumer side of the GPIO, i.e. in the sensor node. 
A 'ti,remote-sources' (or whatever it would be named) property would be 
in the provider, the serializer node. Which one is the correct place to 
configure a thing like this? The REMOTE flag is not quite like the usual 
flags in gpio.h, as the sensor works fine (wrt. the gpio) with or 
without the REMOTE flag.

So maybe 'ti,remote-sources' in the serializer node is better, after 
all, to configure where the serializer gets the GPIOs value...

As for doing this configuration at runtime, with no DT changes... In 
theory that would be possible with custom V4L2 controls to both the 
serializer and the deserializer. But I don't know how the user could 
ever know how to configure it, without having detailed information about 
the HW.

For example, let's say the board generates a frame sync signal (a pulse, 
30 Hz), which is connected to deserializer's GPIO pin X. The deser needs 
to be configured to take the signal from pin X, and forward it to 
serializers 0,1,2,3. Then the serializer needs to be configured to use 
the "remote" gpio X from the deser, and provide it from the serializer's 
GPIO pin, going to the sensor.

  Tomi