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

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

[Guoniu Zhou]

Add back channel GPIO support.
  dt-bindings: media: ti,ds90ub953: Add third cell for GPIO controller
  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 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();
- Link to v1: https://lore.kernel.org/r/20250910-ds90ub953-v1-0-a7813ffbdf11@nxp.com

---
Guoniu Zhou (4):
      dt-bindings: media: ti,ds90ub953: Add third cell for GPIO controller
      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           |  8 +-
 .../bindings/media/i2c/ti,ds90ub960.yaml           |  4 +-
 drivers/media/i2c/ds90ub953.c                      | 99 ++++++++++++----------
 3 files changed, 59 insertions(+), 52 deletions(-)
---
base-commit: 34837c444cd42236b2b43ce871f30d83776a3431
change-id: 20250901-ds90ub953-168628c53a00

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

[PATCH v2 1/4] dt-bindings: media: ti,ds90ub953: Add third cell for GPIO controller

[Guoniu Zhou]

Add third cell for GPIO controller to select GPIO output source. 0 to
select output source from local GPIO data, 1 to select output source
from remote compatible deserializer GPIO data.

Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
---
 Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml | 8 +++++---
 Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml | 4 ++--
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
index 2e129bf573b79e0ca8f25b4ec5fc6ea76c50abd7..de759413a36060d3be6f2c3b67de48ee6e4d29f2 100644
--- a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
@@ -19,10 +19,12 @@ properties:
       - ti,ds90ub971-q1
 
   '#gpio-cells':
-    const: 2
+    const: 3
     description:
       First cell is the GPIO pin number, second cell is the flags. The GPIO pin
       number must be in range of [0, 3].
+      Third cell is GPIO output source(0 for local gpio data, 1 for remote
+      gpio data from remote compatible deserializer).
 
   gpio-controller: true
 
@@ -97,7 +99,7 @@ examples:
         reg = <0x18>;
 
         gpio-controller;
-        #gpio-cells = <2>;
+        #gpio-cells = <3>;
 
         #clock-cells = <0>;
 
@@ -130,7 +132,7 @@ examples:
             compatible = "sony,imx274";
             reg = <0x1a>;
 
-            reset-gpios = <&serializer 0 GPIO_ACTIVE_LOW>;
+            reset-gpios = <&serializer 0 GPIO_ACTIVE_LOW 0>;
 
             clocks = <&serializer>;
             clock-names = "inck";
diff --git a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
index 4dcbd2b039a58edfc57c5cc0bedceefdf158bf0c..4af5e0e3b413a64a4b61344f1da63be7e02a1af8 100644
--- a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
@@ -328,7 +328,7 @@ examples:
               reg = <0x30>;
 
               gpio-controller;
-              #gpio-cells = <2>;
+              #gpio-cells = <3>;
 
               #clock-cells = <0>;
 
@@ -361,7 +361,7 @@ examples:
                   compatible = "sony,imx274";
                   reg = <0x1a>;
 
-                  reset-gpios = <&serializer1 0 GPIO_ACTIVE_LOW>;
+                  reset-gpios = <&serializer1 0 GPIO_ACTIVE_LOW 0>;
 
                   port {
                     sensor_1_out: endpoint {

-- 
2.34.1

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

[Guoniu Zhou]

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 third cell for GPIO controller to select output source.

Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
---
 drivers/media/i2c/ds90ub953.c | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/media/i2c/ds90ub953.c b/drivers/media/i2c/ds90ub953.c
index e3fc9d66970a762d284955f14db48d7105b4b8c4..274972cd21f9f0facd5098b724c0d6c661ce95c2 100644
--- a/drivers/media/i2c/ds90ub953.c
+++ b/drivers/media/i2c/ds90ub953.c
@@ -71,6 +71,7 @@ struct ub953_data {
 	bool			non_continous_clk;
 
 	struct gpio_chip	gpio_chip;
+	bool			gpio_rmten[UB953_NUM_GPIOS];
 
 	struct v4l2_subdev	sd;
 	struct media_pad	pads[2];
@@ -288,13 +289,17 @@ static int ub953_gpio_direction_out(struct gpio_chip *gc, unsigned int offset,
 				    int value)
 {
 	struct ub953_data *priv = gpiochip_get_data(gc);
+	unsigned int 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);
+	val = priv->gpio_rmten[offset]
+	    ? UB953_REG_LOCAL_GPIO_DATA_GPIO_RMTEN(offset)
+	    : value ? UB953_REG_LOCAL_GPIO_DATA_GPIO_OUT_SRC(offset) : 0;
 
+	ret = regmap_update_bits(priv->regmap, UB953_REG_LOCAL_GPIO_DATA,
+				 UB953_REG_LOCAL_GPIO_DATA_GPIO_OUT_SRC(offset) |
+					UB953_REG_LOCAL_GPIO_DATA_GPIO_RMTEN(offset),
+				 val);
 	if (ret)
 		return ret;
 
@@ -320,6 +325,12 @@ static int ub953_gpio_get(struct gpio_chip *gc, unsigned int offset)
 static int ub953_gpio_set(struct gpio_chip *gc, unsigned int offset, int value)
 {
 	struct ub953_data *priv = gpiochip_get_data(gc);
+	struct device *dev = &priv->client->dev;
+
+	if (priv->gpio_rmten[offset]) {
+		dev_err(dev, "GPIO%u be programed to output remote data\n", offset);
+		return -EINVAL;
+	}
 
 	return regmap_update_bits(priv->regmap, UB953_REG_LOCAL_GPIO_DATA,
 				  UB953_REG_LOCAL_GPIO_DATA_GPIO_OUT_SRC(offset),
@@ -330,10 +341,19 @@ 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);
+	u32 line;
+
+	line = gpiospec->args[0];
+	if (line >= UB953_NUM_GPIOS)
+		return -EINVAL;
+
+	priv->gpio_rmten[line] = gpiospec->args[2] ? true : false;
+
 	if (flags)
 		*flags = gpiospec->args[1];
 
-	return gpiospec->args[0];
+	return line;
 }
 
 static int ub953_gpiochip_probe(struct ub953_data *priv)
@@ -363,7 +383,7 @@ static int ub953_gpiochip_probe(struct ub953_data *priv)
 	gc->get = ub953_gpio_get;
 	gc->set = ub953_gpio_set;
 	gc->of_xlate = ub953_gpio_of_xlate;
-	gc->of_gpio_n_cells = 2;
+	gc->of_gpio_n_cells = 3;
 
 	ret = gpiochip_add_data(gc, priv);
 	if (ret) {

-- 
2.34.1

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

[Guoniu Zhou]

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

Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
---
 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 274972cd21f9f0facd5098b724c0d6c661ce95c2..0e3b9796dc1aeae58e626e06e7068b20b40dd213 100644
--- a/drivers/media/i2c/ds90ub953.c
+++ b/drivers/media/i2c/ds90ub953.c
@@ -1337,7 +1337,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
@@ -1346,32 +1348,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) {
@@ -1395,8 +1391,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;
 }
@@ -1411,7 +1405,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 v2 4/4] media: i2c: ds90ub953: use guard() to simplify code

[Guoniu Zhou]

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>
---
 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 0e3b9796dc1aeae58e626e06e7068b20b40dd213..defbd728a0389219397578feee2d3fdc4042bd31 100644
--- a/drivers/media/i2c/ds90ub953.c
+++ b/drivers/media/i2c/ds90ub953.c
@@ -112,20 +112,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 err;
 	}
 
 	*val = v;
 
-out_unlock:
-	mutex_unlock(&priv->reg_lock);
-
+err:
 	if (ret && err)
 		*err = ret;
 
@@ -139,15 +137,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;
 
@@ -184,18 +180,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 err;
 
 	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 err;
 	}
 
 	ret = regmap_read(priv->regmap, UB953_REG_IND_ACC_DATA, &v);
@@ -203,14 +199,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 err;
 	}
 
 	*val = v;
 
-out_unlock:
-	mutex_unlock(&priv->reg_lock);
-
+err:
 	if (ret && err)
 		*err = ret;
 
@@ -226,18 +220,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 err;
 
 	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 err;
 	}
 
 	ret = regmap_write(priv->regmap, UB953_REG_IND_ACC_DATA, val);
@@ -247,9 +241,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);
-
+err:
 	if (ret && err)
 		*err = ret;
 

-- 
2.34.1

Re: [PATCH v2 1/4] dt-bindings: media: ti,ds90ub953: Add third cell for GPIO controller

[Rob Herring]

On Thu, Sep 11, 2025 at 04:44:22PM +0800, Guoniu Zhou wrote:
> Add third cell for GPIO controller to select GPIO output source. 0 to
> select output source from local GPIO data, 1 to select output source
> from remote compatible deserializer GPIO data.
> 
> Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> ---
>  Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml | 8 +++++---
>  Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml | 4 ++--
>  2 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
> index 2e129bf573b79e0ca8f25b4ec5fc6ea76c50abd7..de759413a36060d3be6f2c3b67de48ee6e4d29f2 100644
> --- a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
> @@ -19,10 +19,12 @@ properties:
>        - ti,ds90ub971-q1
>  
>    '#gpio-cells':
> -    const: 2
> +    const: 3
>      description:
>        First cell is the GPIO pin number, second cell is the flags. The GPIO pin
>        number must be in range of [0, 3].
> +      Third cell is GPIO output source(0 for local gpio data, 1 for remote
> +      gpio data from remote compatible deserializer).

Changing the cell size is an ABI break unless 3 cells was supported in 
OS before which I don't think it would be given GPIO cell meaning is 
pretty standardized. You could just all a flag to the 2nd cell I think. 
Or if 0-3 are local GPIOs, then make remote ones a different range.

Rob