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

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

[Guoniu Zhou]

Add back channel GPIO support.
  dt-bindings: media: ti,ds90ub953: Add new range for remote GPIO data
  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 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 new range for remote GPIO data
      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 +-
 drivers/media/i2c/ds90ub953.c                      | 109 ++++++++++++---------
 2 files changed, 66 insertions(+), 51 deletions(-)
---
base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
change-id: 20250901-ds90ub953-168628c53a00

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

[PATCH v5 1/4] dt-bindings: media: ti,ds90ub953: Add new range for remote GPIO data

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

Make a different GPIO range for remote ones.

Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
---
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 | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
index 2e129bf573b79e0ca8f25b4ec5fc6ea76c50abd7..143d546ecb482d2eb78a113b1a57afcbff9cca01 100644
--- a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
@@ -20,9 +20,13 @@ properties:
 
   '#gpio-cells':
     const: 2
-    description:
+    description: >
       First cell is the GPIO pin number, second cell is the flags. The GPIO pin
-      number must be in range of [0, 3].
+      number must be in range of [0, 3] when enabled as an input.
+
+      If enabled as an output, the GPIO pin number must be in range of [0, 7].
+      [0, 3] are for local GPIO data and [4, 7] for GPIO data coming from
+      remote compatible deserializer.
 
   gpio-controller: true
 

-- 
2.34.1

[PATCH v5 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.

Make GPIO range from 0-3 to 0-7 to support this case. 0-3 for local GPIO
and 4-7 for remote GPIO data.

Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
---
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 | 42 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 8 deletions(-)

diff --git a/drivers/media/i2c/ds90ub953.c b/drivers/media/i2c/ds90ub953.c
index daefdb108fbf402453baeaccc61d1f3530521d74..14dd0aa4cc6ceba66a8c3545c7d7d19694007431 100644
--- a/drivers/media/i2c/ds90ub953.c
+++ b/drivers/media/i2c/ds90ub953.c
@@ -32,7 +32,9 @@
 #define UB953_PAD_SINK			0
 #define UB953_PAD_SOURCE		1
 
-#define UB953_NUM_GPIOS			4
+#define UB953_NUM_LOCAL_GPIOS		4
+#define UB953_NUM_REMOTE_GPIOS		4
+#define UB953_NUM_GPIOS			(UB953_NUM_LOCAL_GPIOS + UB953_NUM_REMOTE_GPIOS)
 
 #define UB953_DEFAULT_CLKOUT_RATE	25000000UL
 
@@ -268,7 +270,7 @@ static int ub953_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
 	if (ret)
 		return ret;
 
-	if (v & UB953_REG_GPIO_INPUT_CTRL_INPUT_EN(offset))
+	if (v & UB953_REG_GPIO_INPUT_CTRL_INPUT_EN((offset % UB953_NUM_LOCAL_GPIOS)))
 		return GPIO_LINE_DIRECTION_IN;
 	else
 		return GPIO_LINE_DIRECTION_OUT;
@@ -277,6 +279,12 @@ static int ub953_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
 static int ub953_gpio_direction_in(struct gpio_chip *gc, unsigned int offset)
 {
 	struct ub953_data *priv = gpiochip_get_data(gc);
+	struct device *dev = &priv->client->dev;
+
+	if (offset > UB953_NUM_LOCAL_GPIOS) {
+		dev_err(dev, "GPIO%u can't be programed as input\n", offset);
+		return -EINVAL;
+	}
 
 	return regmap_update_bits(priv->regmap, UB953_REG_GPIO_INPUT_CTRL,
 				  UB953_REG_GPIO_INPUT_CTRL_INPUT_EN(offset) |
@@ -288,10 +296,14 @@ 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 pin;
 	int ret;
 
+	pin = offset % UB953_NUM_LOCAL_GPIOS;
+
 	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_OUT_SRC(pin) |
+				 UB953_REG_LOCAL_GPIO_DATA_GPIO_RMTEN(pin),
 				 value ? UB953_REG_LOCAL_GPIO_DATA_GPIO_OUT_SRC(offset) :
 					 0);
 
@@ -299,9 +311,9 @@ static int ub953_gpio_direction_out(struct gpio_chip *gc, unsigned int offset,
 		return ret;
 
 	return regmap_update_bits(priv->regmap, UB953_REG_GPIO_INPUT_CTRL,
-				  UB953_REG_GPIO_INPUT_CTRL_INPUT_EN(offset) |
-					  UB953_REG_GPIO_INPUT_CTRL_OUT_EN(offset),
-				  UB953_REG_GPIO_INPUT_CTRL_OUT_EN(offset));
+				  UB953_REG_GPIO_INPUT_CTRL_INPUT_EN(pin) |
+					  UB953_REG_GPIO_INPUT_CTRL_OUT_EN(pin),
+				  UB953_REG_GPIO_INPUT_CTRL_OUT_EN(pin));
 }
 
 static int ub953_gpio_get(struct gpio_chip *gc, unsigned int offset)
@@ -314,12 +326,19 @@ static int ub953_gpio_get(struct gpio_chip *gc, unsigned int offset)
 	if (ret)
 		return ret;
 
-	return !!(v & UB953_REG_GPIO_PIN_STS_GPIO_STS(offset));
+	return !!(v & UB953_REG_GPIO_PIN_STS_GPIO_STS((offset % UB953_NUM_LOCAL_GPIOS)));
 }
 
 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 (offset > UB953_NUM_LOCAL_GPIOS) {
+		dev_err(dev, "GPIO%u be programed to output remote data\n",
+			offset % UB953_NUM_LOCAL_GPIOS);
+		return -EINVAL;
+	}
 
 	return regmap_update_bits(priv->regmap, UB953_REG_LOCAL_GPIO_DATA,
 				  UB953_REG_LOCAL_GPIO_DATA_GPIO_OUT_SRC(offset),
@@ -330,6 +349,9 @@ static int ub953_gpio_of_xlate(struct gpio_chip *gc,
 			       const struct of_phandle_args *gpiospec,
 			       u32 *flags)
 {
+	if (gpiospec->args[0] >= UB953_NUM_GPIOS)
+		return -EINVAL;
+
 	if (flags)
 		*flags = gpiospec->args[1];
 
@@ -632,7 +654,11 @@ static int ub953_log_status(struct v4l2_subdev *sd)
 	if (ret)
 		return ret;
 
-	for (i = 0; i < UB953_NUM_GPIOS; i++) {
+	/*
+	 * Only log GPIO 0-3 state since remote GPIO 4-7 pins reuse local
+	 * GPIO 0-3 when enabled as an output.
+	 */
+	for (i = 0; i < UB953_NUM_LOCAL_GPIOS; i++) {
 		dev_info(dev,
 			 "GPIO%u: remote: %u is_input: %u is_output: %u val: %u sts: %u\n",
 			 i,

-- 
2.34.1

[PATCH v5 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>
Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
---
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 14dd0aa4cc6ceba66a8c3545c7d7d19694007431..a85c6a9b64070491db161ca1586179dba9c69cb0 100644
--- a/drivers/media/i2c/ds90ub953.c
+++ b/drivers/media/i2c/ds90ub953.c
@@ -1345,7 +1345,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
@@ -1354,32 +1356,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) {
@@ -1403,8 +1399,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;
 }
@@ -1419,7 +1413,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 v5 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>
---
 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 a85c6a9b64070491db161ca1586179dba9c69cb0..3a459687aefce05ac025a517c8d4cc6d76cd7293 100644
--- a/drivers/media/i2c/ds90ub953.c
+++ b/drivers/media/i2c/ds90ub953.c
@@ -113,20 +113,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;
 
@@ -140,15 +138,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;
 
@@ -185,18 +181,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);
@@ -204,14 +200,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;
 
@@ -227,18 +221,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);
@@ -248,9 +242,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 v5 1/4] dt-bindings: media: ti,ds90ub953: Add new range for remote GPIO data

[Rob Herring (Arm)]

On Sat, 28 Feb 2026 14:18:19 +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.
> 
> Make a different GPIO range for remote ones.
> 
> Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> ---
> 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 | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH v5 1/4] dt-bindings: media: ti,ds90ub953: Add new range for remote GPIO data

[Tomi Valkeinen]

Jo,

On 28/02/2026 08:18, 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.
> 
> Make a different GPIO range for remote ones.
> 
> Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> ---
> 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 | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
> index 2e129bf573b79e0ca8f25b4ec5fc6ea76c50abd7..143d546ecb482d2eb78a113b1a57afcbff9cca01 100644
> --- a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
> @@ -20,9 +20,13 @@ properties:
>  
>    '#gpio-cells':
>      const: 2
> -    description:
> +    description: >
>        First cell is the GPIO pin number, second cell is the flags. The GPIO pin
> -      number must be in range of [0, 3].
> +      number must be in range of [0, 3] when enabled as an input.
> +
> +      If enabled as an output, the GPIO pin number must be in range of [0, 7].
> +      [0, 3] are for local GPIO data and [4, 7] for GPIO data coming from
> +      remote compatible deserializer.
I see there has been some discussions about this in the earlier
versions. I guess this works, but feels a bit hacky, makes the driver a
bit odd, and assumes that no consumer ever uses the same gpio from the
0-3 range and 4-7 range.

Did you try the approach where a new flag would be used to indicate the
remote gpio functionality? Any problems there? I'm not sure what's the
policy for adding custom gpio flags, though.

 Tomi

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

[Tomi Valkeinen]

Hi,

On 28/02/2026 08:18, Guoniu Zhou wrote:
> 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>
> Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> ---
> Changes in v2:
> - Move PTR_ERR() in dev_err_probe();
> ---

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

 Tomi

>  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 14dd0aa4cc6ceba66a8c3545c7d7d19694007431..a85c6a9b64070491db161ca1586179dba9c69cb0 100644
> --- a/drivers/media/i2c/ds90ub953.c
> +++ b/drivers/media/i2c/ds90ub953.c
> @@ -1345,7 +1345,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
> @@ -1354,32 +1356,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) {
> @@ -1403,8 +1399,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;
>  }
> @@ -1419,7 +1413,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 = {
> 

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

[Tomi Valkeinen]

Hi,

On 28/02/2026 08:18, Guoniu Zhou wrote:
> From: Guoniu Zhou <guoniu.zhou@nxp.com>
> 
> Use guard() to simplify mutex locking. No functional change.

That's not strictly true, as the unlock will happen later with this
patch. Still, this cleanup makes sense.

> 
> 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 a85c6a9b64070491db161ca1586179dba9c69cb0..3a459687aefce05ac025a517c8d4cc6d76cd7293 100644
> --- a/drivers/media/i2c/ds90ub953.c
> +++ b/drivers/media/i2c/ds90ub953.c
> @@ -113,20 +113,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:

I think the label should be just "out", as this is not an error path
(even if we jump to the label when handling an error). Similar comment
to the below cases.

>  	if (ret && err)
>  		*err = ret;
>  
> @@ -140,15 +138,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;
>  
> @@ -185,18 +181,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);
> @@ -204,14 +200,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;
>  
> @@ -227,18 +221,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);
> @@ -248,9 +242,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;
>  
> 

RE: [PATCH v5 1/4] dt-bindings: media: ti,ds90ub953: Add new range for remote GPIO data

[G.N. Zhou (OSS)]

Hi Tomi

Thanks for your review.

> -----Original Message-----
> From: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> Sent: Wednesday, March 11, 2026 4:37 PM
> To: G.N. Zhou (OSS) <guoniu.zhou@oss.nxp.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>
> Cc: linux-media@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; imx@lists.linux.dev; G.N. Zhou
> <guoniu.zhou@nxp.com>
> Subject: Re: [PATCH v5 1/4] dt-bindings: media: ti,ds90ub953: Add new range
> for remote GPIO data
> 
> Jo,
> 
> On 28/02/2026 08:18, 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.
> >
> > Make a different GPIO range for remote ones.
> >
> > Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> > ---
> > 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 | 8
> > ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
> > b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
> > index
> >
> 2e129bf573b79e0ca8f25b4ec5fc6ea76c50abd7..143d546ecb482d2eb78a113b
> 1a57
> > afcbff9cca01 100644
> > --- a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
> > +++ b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
> > @@ -20,9 +20,13 @@ properties:
> >
> >    '#gpio-cells':
> >      const: 2
> > -    description:
> > +    description: >
> >        First cell is the GPIO pin number, second cell is the flags. The GPIO pin
> > -      number must be in range of [0, 3].
> > +      number must be in range of [0, 3] when enabled as an input.
> > +
> > +      If enabled as an output, the GPIO pin number must be in range of [0, 7].
> > +      [0, 3] are for local GPIO data and [4, 7] for GPIO data coming from
> > +      remote compatible deserializer.
> I see there has been some discussions about this in the earlier versions. I guess
> this works, but feels a bit hacky, makes the driver a bit odd, and assumes that
> no consumer ever uses the same gpio from the
> 0-3 range and 4-7 range.
> 
> Did you try the approach where a new flag would be used to indicate the
> remote gpio functionality? Any problems there? I'm not sure what's the policy
> for adding custom gpio flags, though.

No, but I could try.

> 
>  Tomi

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

[G.N. Zhou (OSS)]

Hi Tomi,

Thanks for your review.

> -----Original Message-----
> From: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> Sent: Wednesday, March 11, 2026 4:45 PM
> To: G.N. Zhou (OSS) <guoniu.zhou@oss.nxp.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>
> Cc: linux-media@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; imx@lists.linux.dev; G.N. Zhou
> <guoniu.zhou@nxp.com>
> Subject: Re: [PATCH v5 4/4] media: i2c: ds90ub953: use guard() to simplify code
> 
> Hi,
> 
> On 28/02/2026 08:18, Guoniu Zhou wrote:
> > From: Guoniu Zhou <guoniu.zhou@nxp.com>
> >
> > Use guard() to simplify mutex locking. No functional change.
> 
> That's not strictly true, as the unlock will happen later with this patch. Still, this
> cleanup makes sense.
> 
> >
> > 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
> >
> a85c6a9b64070491db161ca1586179dba9c69cb0..3a459687aefce05ac025a51
> 7c8d4
> > cc6d76cd7293 100644
> > --- a/drivers/media/i2c/ds90ub953.c
> > +++ b/drivers/media/i2c/ds90ub953.c
> > @@ -113,20 +113,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:
> 
> I think the label should be just "out", as this is not an error path (even if we
> jump to the label when handling an error). Similar comment to the below
> cases.
> 

Agree, will update in next version.

> >  	if (ret && err)
> >  		*err = ret;
> >
> > @@ -140,15 +138,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;
> >
> > @@ -185,18 +181,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);
> @@
> > -204,14 +200,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;
> >
> > @@ -227,18 +221,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);
> @@
> > -248,9 +242,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;
> >
> >