media: ov9282: Add external trigger mode support by omeredemen · Pull Request #6954 · raspberrypi/linux

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

 	/* 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


 	/* 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

+
+	/* 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


 	/* 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.

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

+			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; }

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

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

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 <ofedemen@lectrontech.com>

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 <ofedemen@lectrontech.com>

pelwell · 2025-10-15T14:36:04Z

FYI we don't (always?) get notifications of branch updates, so adding a comment once you've done so helps to avoid long delays.

I think this is good now. Feel free to merge if you agree, @6by9.

kernel: Add config settings and i2c-sensor support for the TMP117 See: raspberrypi/linux#7083 kernel: Upstream patches for TC358743 colour swap See: raspberrypi/linux#7070 kernel: media: ov9282: Add external trigger mode support See: raspberrypi/linux#6954 kernel: Improve the RP1 DMA channel allocation scheme See: raspberrypi/linux#7089

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

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

Omer Faruk Edemen added 2 commits September 22, 2025 07:43

arm: dts: overlays: Add external trigger mode support

0604690

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 <ofedemen@lectrontech.com>

omeredemen force-pushed the rpi-6.12.y branch from 9ecf861 to 0604690 Compare September 22, 2025 04:44

pelwell approved these changes Oct 15, 2025

View reviewed changes

6by9 merged commit f02b096 into raspberrypi:rpi-6.12.y Oct 15, 2025
12 checks passed

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!

pelwell commented Oct 15, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants