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.

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]> Signed-off-by: omer <[email protected]>

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 <[email protected]>

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.

6by9 · 2025-09-01T10:38:39Z

drivers/media/i2c/ov9282.c

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


Again a whitespace change, and this one breaks the linux-media coding standard of max 80 chars per line. https://www.kernel.org/doc/html/latest/driver-api/media/maintainer-entry-profile.html#coding-style-addendum

ret = ov9282_write_reg(ov9282, OV9282_REG_MODE_SELECT, 1, OV9282_MODE_STREAMING);

would be permitted and cleaner, but again it shouldn't be mixed in with a functional change.

6by9 · 2025-09-01T10:38:54Z

drivers/media/i2c/ov9282.c

@@ -964,13 +1023,12 @@ static int ov9282_start_streaming(struct ov9282 *ov9282)
 			{OV9282_REG_ANA_CORE_2, OV9282_ANA_CORE2_RAW8},
 		}
 	};
-	const struct ov9282_reg_list *reg_list;
+	struct ov9282_reg_list *reg_list;


Why have we lost the const?
And then we regain it in your second patch.

6by9 · 2025-09-01T10:43:24Z

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

 /dts-v1/;
 /plugin/;

+#include <dt-bindings/gpio/gpio.h>


Why is this required? I don't see any references in the overlay to defines that it provides.

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

Omer Faruk Edemen and others added 2 commits August 29, 2025 07:45

media: ov9282: Add external trigger mode support

540bb3c

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]> Signed-off-by: omer <[email protected]>

media: i2c: ov9282: Add external FSIN trigger snapshot mode

be0e8f1

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 <[email protected]>

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

media: ov9282: Add external trigger mode support #6954

media: ov9282: Add external trigger mode support #6954

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

Uh oh!

6by9 Aug 12, 2025

Uh oh!

6by9 Aug 12, 2025

Uh oh!

6by9 Sep 1, 2025

Uh oh!

6by9 Aug 12, 2025

Uh oh!

stalinbeltran commented Aug 28, 2025

Uh oh!

omeredemen commented Aug 29, 2025

Uh oh!

6by9 left a comment

Uh oh!

6by9 Sep 1, 2025

Uh oh!

6by9 Sep 1, 2025

Uh oh!

6by9 Sep 1, 2025

Uh oh!

6by9 Sep 1, 2025

Uh oh!

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!