[PATCH v2 0/2] ov8856: Add V4L2 device tree properties support

[PATCH v2 0/2] ov8856: Add V4L2 device tree properties support

[Alexander Koskovich]

Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
---
Changes in v2:
- Updated bindings to address Krzysztof's review (only allow orientation & rotation)
- Address Jacopo's review by removing redundant v4l2_ctrl_handler_free()
- Link to v1: https://lore.kernel.org/r/20260307-ov8856-v4l2-props-v1-0-7677b4c658e4@pm.me

---
Alexander Koskovich (2):
      media: dt-bindings: ovti,ov8856: Allow orientation & rotation props
      media: i2c: ov8856: parse and register V4L2 device tree properties

 .../devicetree/bindings/media/i2c/ovti,ov8856.yaml          |  7 +++++++
 drivers/media/i2c/ov8856.c                                  | 13 ++++++++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)
---
base-commit: 1f318b96cc84d7c2ab792fcc0bfd42a7ca890681
change-id: 20260307-ov8856-v4l2-props-def87d7854b1

Best regards,
-- 
Alexander Koskovich <akoskovich@pm.me>

[PATCH v2 1/2] media: dt-bindings: ovti,ov8856: Allow orientation & rotation props

[Alexander Koskovich]

Allow the orientation and rotation properties from video-interface-devices
to be specified. The sensor can be front or rear facing and can be mounted
at any rotation.

Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
---
 Documentation/devicetree/bindings/media/i2c/ovti,ov8856.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov8856.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov8856.yaml
index fa71f24823f2..24d4161a293b 100644
--- a/Documentation/devicetree/bindings/media/i2c/ovti,ov8856.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov8856.yaml
@@ -18,6 +18,9 @@ description: |-
   through I2C and two-wire SCCB. The sensor output is available via CSI-2
   serial data output (up to 4-lane).
 
+allOf:
+  - $ref: /schemas/media/video-interface-devices.yaml#
+
 properties:
   compatible:
     const: ovti,ov8856
@@ -57,6 +60,10 @@ properties:
       This corresponds to the hardware pin XSHUTDOWN which is physically
       active low.
 
+  orientation: true
+
+  rotation: true
+
   port:
     $ref: /schemas/graph.yaml#/$defs/port-base
     additionalProperties: false

-- 
2.53.0

[PATCH v2 2/2] media: i2c: ov8856: parse and register V4L2 device tree properties

[Alexander Koskovich]

Parse V4L2 device tree properties and register controls for them.

Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
---
 drivers/media/i2c/ov8856.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov8856.c b/drivers/media/i2c/ov8856.c
index e2998cfa0d18..9d7d16ba8911 100644
--- a/drivers/media/i2c/ov8856.c
+++ b/drivers/media/i2c/ov8856.c
@@ -1887,12 +1887,14 @@ static const struct v4l2_ctrl_ops ov8856_ctrl_ops = {
 
 static int ov8856_init_controls(struct ov8856 *ov8856)
 {
+	struct i2c_client *client = v4l2_get_subdevdata(&ov8856->sd);
+	struct v4l2_fwnode_device_properties props;
 	struct v4l2_ctrl_handler *ctrl_hdlr;
 	s64 exposure_max, h_blank;
 	int ret;
 
 	ctrl_hdlr = &ov8856->ctrl_handler;
-	ret = v4l2_ctrl_handler_init(ctrl_hdlr, 8);
+	ret = v4l2_ctrl_handler_init(ctrl_hdlr, 10);
 	if (ret)
 		return ret;
 
@@ -1954,6 +1956,15 @@ static int ov8856_init_controls(struct ov8856 *ov8856)
 	if (ctrl_hdlr->error)
 		return ctrl_hdlr->error;
 
+	ret = v4l2_fwnode_device_parse(&client->dev, &props);
+	if (ret)
+		return ret;
+
+	ret = v4l2_ctrl_new_fwnode_properties(ctrl_hdlr, &ov8856_ctrl_ops,
+					      &props);
+	if (ret)
+		return ret;
+
 	ov8856->sd.ctrl_handler = ctrl_hdlr;
 
 	return 0;

-- 
2.53.0

Re: [PATCH v2 2/2] media: i2c: ov8856: parse and register V4L2 device tree properties

[Sakari Ailus]

Hi Alexander,

On Tue, Mar 10, 2026 at 04:21:34AM +0000, Alexander Koskovich wrote:
> Parse V4L2 device tree properties and register controls for them.
> 
> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> ---
>  drivers/media/i2c/ov8856.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/ov8856.c b/drivers/media/i2c/ov8856.c
> index e2998cfa0d18..9d7d16ba8911 100644
> --- a/drivers/media/i2c/ov8856.c
> +++ b/drivers/media/i2c/ov8856.c
> @@ -1887,12 +1887,14 @@ static const struct v4l2_ctrl_ops ov8856_ctrl_ops = {
>  
>  static int ov8856_init_controls(struct ov8856 *ov8856)
>  {
> +	struct i2c_client *client = v4l2_get_subdevdata(&ov8856->sd);
> +	struct v4l2_fwnode_device_properties props;
>  	struct v4l2_ctrl_handler *ctrl_hdlr;
>  	s64 exposure_max, h_blank;
>  	int ret;
>  
>  	ctrl_hdlr = &ov8856->ctrl_handler;
> -	ret = v4l2_ctrl_handler_init(ctrl_hdlr, 8);
> +	ret = v4l2_ctrl_handler_init(ctrl_hdlr, 10);
>  	if (ret)
>  		return ret;
>  
> @@ -1954,6 +1956,15 @@ static int ov8856_init_controls(struct ov8856 *ov8856)
>  	if (ctrl_hdlr->error)
>  		return ctrl_hdlr->error;
>  
> +	ret = v4l2_fwnode_device_parse(&client->dev, &props);
> +	if (ret)
> +		return ret;
> +
> +	ret = v4l2_ctrl_new_fwnode_properties(ctrl_hdlr, &ov8856_ctrl_ops,
> +					      &props);
> +	if (ret)
> +		return ret;
> +

It's not a bug introduced in this patch, but freeing the control handler on
error path was broken to begin with: it was never set to the sub-device on
error so the caller can't release it.

Could you fix that before this patch so we can get it backported?

I think it'd be cleanest to add a new label for freeing it and change all
existing returns to gotos.

>  	ov8856->sd.ctrl_handler = ctrl_hdlr;
>  
>  	return 0;
> 

-- 
Kind regards,

Sakari Ailus

Re: [PATCH v2 1/2] media: dt-bindings: ovti,ov8856: Allow orientation & rotation props

[Krzysztof Kozlowski]

On Tue, Mar 10, 2026 at 04:21:31AM +0000, Alexander Koskovich wrote:
> Allow the orientation and rotation properties from video-interface-devices
> to be specified. The sensor can be front or rear facing and can be mounted
> at any rotation.
> 
> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> ---
>  Documentation/devicetree/bindings/media/i2c/ovti,ov8856.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov8856.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov8856.yaml
> index fa71f24823f2..24d4161a293b 100644
> --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov8856.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov8856.yaml
> @@ -18,6 +18,9 @@ description: |-
>    through I2C and two-wire SCCB. The sensor output is available via CSI-2
>    serial data output (up to 4-lane).
>  
> +allOf:
> +  - $ref: /schemas/media/video-interface-devices.yaml#
> +
>  properties:
>    compatible:
>      const: ovti,ov8856
> @@ -57,6 +60,10 @@ properties:
>        This corresponds to the hardware pin XSHUTDOWN which is physically
>        active low.

I was fine with previous patch if my assumptions stated there were correct.

>  
> +  orientation: true
> +

if there is going to be new version, then drop blank line here.

> +  rotation: true
> +

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof