media: ov9282: Add external trigger mode support #6954

omeredemen · 2025-07-11T08:34:52Z

Adds DT property trigger-mode to enable FSIN-triggered frame capture. Includes overlay and README update for ov9281_trig.

#5349 (comment)

pelwell · 2025-07-15T10:30:37Z

Please squash your ov9282 commits into one, then split that in two - one for the driver changes and one for the overlay changes. You can then run scripts/checkpatch.pl -g HEAD^ and scripts/checkpatch.pl -g HEAD to check for remaining indentation complaints. It would also be nice if the Signed-off-by lines weren't "Your Name". After all that, use git push -f ... to update your branch (and hence the PR).

With those changes, I presume you are happy with this @6by9?

pelwell · 2025-08-12T12:22:20Z

Any comments or progress on this?

6by9

Sorry for the delay in reviewing.

6by9 · 2025-08-12T12:56:00Z

drivers/media/i2c/ov9282.c

 	/* Start streaming */
 	ret = ov9282_write_reg(ov9282, OV9282_REG_MODE_SELECT,
-			       1, OV9282_MODE_STREAMING);
+			1, OV9282_MODE_STREAMING);


Odd whitespace change for no reason

6by9 · 2025-08-12T12:56:30Z

drivers/media/i2c/ov9282.c

@@ -992,15 +1060,25 @@ static int ov9282_start_streaming(struct ov9282 *ov9282)
 	}

 	/* Setup handler will write actual exposure and gain */
-	ret =  __v4l2_ctrl_handler_setup(ov9282->sd.ctrl_handler);
+	ret = __v4l2_ctrl_handler_setup(ov9282->sd.ctrl_handler);


Whitespace change

Looking again I'll acknowledge that it's wrong in the original, but changing it should be a separate patch.

6by9 · 2025-08-12T12:57:18Z

drivers/media/i2c/ov9282.c

+
+	/* 1 frame per trigger */
+        ov9282_write_reg(ov9282, OV9282_REG_NUM_FRAME_ON_TRIG, 1, 0x01);
+        if (ret) {


You've not saved the result of ov9282_write_reg to ret, so this will never fail.

Ditto subsequent calls.

stalinbeltran · 2025-08-28T10:53:03Z

I need external trigger in order to capture laser light reflected from a moving mirror, so I tried this code with:

sudo rpi-update pulls/6954

It works. Tested with CAM-MIPIOV9281V2 (Innomaker) on a Raspberry 5.
If I can help somehow, please let me know. This should be made available in the official kernel (not everybody can find this pull request)

Thank you.

omeredemen · 2025-08-29T05:00:17Z

I need external trigger in order to capture laser light reflected from a moving mirror, so I tried this code with:

sudo rpi-update pulls/6954

It works. Tested with CAM-MIPIOV9281V2 (Innomaker) on a Raspberry 5. If I can help somehow, please let me know. This should be made available in the official kernel (not everybody can find this pull request)

Thank you.

Sorry for the latency. @stalinbeltran i am glad it worked for you.

6by9

Sorry but this seems to be worse than last time.

You haven't address pelwell's comment

Please squash your ov9282 commits into one, then split that in two - one for the driver changes and one for the overlay changes.

We have a number of whitespace changes. One is correct, but whitespace cleanups should never be merged into the same commit as functional changes.

6by9 · 2025-09-01T10:29:04Z

drivers/media/i2c/ov9282.c

@@ -992,15 +1060,25 @@ static int ov9282_start_streaming(struct ov9282 *ov9282)
 	}

 	/* Setup handler will write actual exposure and gain */
-	ret =  __v4l2_ctrl_handler_setup(ov9282->sd.ctrl_handler);
+	ret = __v4l2_ctrl_handler_setup(ov9282->sd.ctrl_handler);


Looking again I'll acknowledge that it's wrong in the original, but changing it should be a separate patch.

drivers/media/i2c/ov9282.c

arch/arm/boot/dts/overlays/ov9281-overlay.dts

omeredemen · 2025-09-08T07:16:58Z

So i think i had made some changes in the original. and didn't squash the commits with the correct changes. i just changed the necessary part for the trigger mode and didn't make any changes on the other parts.

6by9

One slight nasty here.

Seeing as this won't get upstreamed in this form I am prepared to accept it as is, but it would be nicer if you'd update.

Thanks.

6by9 · 2025-09-08T13:26:43Z

drivers/media/i2c/ov9282.c

+			dev_err(ov9282->dev, "failed to config external trigger mode");
+			return ret;
+		}
+		return 0;


There's a bit of a gotcha here with this early bail out that I hadn't noticed it in the previous review pass.

My preference would be to keep setting OV9282_REG_MODE_SELECT here in both cases, ie.

/* Configure FSIN external trigger mode */ if (ov9282->trigger_mode > 0) { ret = ov9282_apply_trigger_config(ov9282); if (ret) { dev_err(ov9282->dev, "failed to config external trigger mode"); return ret; } /* stay in standby mode and wait for trigger signal */ ret = ov9282_write_reg(ov9282, OV9282_REG_MODE_SELECT, 1, OV9282_MODE_STANDBY); } else { /* Start streaming */ ret = ov9282_write_reg(ov9282, OV9282_REG_MODE_SELECT, 1, OV9282_MODE_STREAMING); } if (ret) dev_err(ov9282->dev, "fail to start streaming"); return ret; }

This patch adds support for external FSIN-triggered snapshot mode to the OmniVision OV9282 sensor driver. It enables frame capture synchronized with an external hardware trigger signal. Signed-off-by: Omer Faruk Edemen <[email protected]>

Adds DT property `trigger-mode` to enable FSIN-triggered frame capture. Includes overlay and README update for ov9281_trig. Signed-off-by: Omer Faruk Edemen <[email protected]>

6by9 · 2025-09-08T14:29:01Z

drivers/media/i2c/ov9282.c

+		return ret;
+
+	/* stay standby mode and wait for trigger signal */
+	ret = ov9282_write_reg(ov9282, OV9282_REG_MODE_SELECT,


Sorry I didn't say it explicitly, but we now have a duplication here. Just drop this write.

omeredemen force-pushed the rpi-6.12.y branch from c68ef01 to 3c45a63 Compare July 11, 2025 09:58

6by9 requested changes Aug 12, 2025

View reviewed changes

omeredemen force-pushed the rpi-6.12.y branch from fb1302e to be0e8f1 Compare August 29, 2025 04:55

6by9 requested changes Sep 1, 2025

View reviewed changes

omeredemen force-pushed the rpi-6.12.y branch from be0e8f1 to 9a8ea63 Compare September 8, 2025 07:12

6by9 reviewed Sep 8, 2025

View reviewed changes

Omer Faruk Edemen added 2 commits September 8, 2025 17:11

arm: dts: overlays: Add external trigger mode support

9ecf861

Adds DT property `trigger-mode` to enable FSIN-triggered frame capture. Includes overlay and README update for ov9281_trig. Signed-off-by: Omer Faruk Edemen <[email protected]>

omeredemen force-pushed the rpi-6.12.y branch from 9a8ea63 to 9ecf861 Compare September 8, 2025 14:15

6by9 reviewed Sep 8, 2025

View reviewed changes

media: ov9282: Add external trigger mode support #6954

Are you sure you want to change the base?

media: ov9282: Add external trigger mode support #6954

Conversation

omeredemen commented Jul 11, 2025

Uh oh!

pelwell commented Jul 15, 2025

Uh oh!

pelwell commented Aug 12, 2025

Uh oh!

6by9 left a comment

Choose a reason for hiding this comment

Uh oh!

6by9 Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

6by9 Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

6by9 Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

6by9 Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

stalinbeltran commented Aug 28, 2025

Uh oh!

omeredemen commented Aug 29, 2025

Uh oh!

6by9 left a comment

Choose a reason for hiding this comment

Uh oh!

6by9 Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

omeredemen commented Sep 8, 2025

Uh oh!

6by9 left a comment

Choose a reason for hiding this comment

Uh oh!

6by9 Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

6by9 Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!