Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions arch/arm/boot/dts/overlays/README
Original file line number Diff line number Diff line change
Expand Up @@ -3718,6 +3718,10 @@ Params: rotation Mounting rotation of the camera sensor (0 or
clk-continuous Switch to continuous mode on the CSI clock lane,
which increases the maximum frame rate slightly.
Appears not to work on Pi3.
trigger-mode Enable external trigger mode (0 = normal,
1 = triggered). In this mode, the sensor outputs
a frame only when triggered by a rising edge
on the FSIN input pin.


Name: papirus
Expand Down
4 changes: 2 additions & 2 deletions arch/arm/boot/dts/overlays/ov9281-overlay.dts
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
// SPDX-License-Identifier: GPL-2.0-only
// Definitions for OV9281 camera module on VC I2C bus
// Definitions for OV9281_trig camera module on VC I2C bus
/dts-v1/;
/plugin/;

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

/{
compatible = "brcm,bcm2835";
Expand Down Expand Up @@ -96,6 +95,7 @@
<&reg_frag>, "target:0=",<&cam0_reg>;
arducam = <0>, "+6";
clk-continuous = <0>, "-7-8";
trigger-mode = <&cam_node>, "trigger-mode:0";
};
};

Expand Down
1 change: 1 addition & 0 deletions arch/arm/boot/dts/overlays/ov9281.dtsi
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ cam_node: ov9281@60 {

rotation = <0>;
orientation = <2>;
trigger-mode = <0>;

port {
cam_endpoint: endpoint {
Expand Down
83 changes: 77 additions & 6 deletions drivers/media/i2c/ov9282.c
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,14 @@
#define OV9282_REG_MIPI_CTRL00 0x4800
#define OV9282_GATED_CLOCK BIT(5)

/* Trigger mode registers */
#define OV9282_REG_POWER_CTRL 0x4F00
#define OV9282_REG_LOW_POWER_MODE_CTRL 0x3030
#define OV9282_REG_NUM_FRAME_ON_TRIG 0x303F
#define OV9282_REG_SLEEP_PERIOD_CTRL0 0x302C
#define OV9282_REG_SLEEP_PERIOD_CTRL3 0x302F
#define OV9282_REG_TIMING_23 0x3823

/* Input clock rate */
#define OV9282_INCLK_RATE 24000000

Expand Down Expand Up @@ -187,6 +195,7 @@ struct ov9282 {
const struct ov9282_mode *cur_mode;
u32 code;
struct mutex mutex;
int trigger_mode;
};

static const s64 link_freq[] = {
Expand Down Expand Up @@ -947,6 +956,56 @@ static int ov9282_get_selection(struct v4l2_subdev *sd,
return -EINVAL;
}

/**
* ov9282_apply_trigger_config() - Configure sensor for FSIN external trigger mode
* @ov9282: pointer to ov9282 device
*
* Return: 0 on success, error code otherwise.
*/
static int ov9282_apply_trigger_config(struct ov9282 *ov9282)
{
int ret;

ret = ov9282_write_reg(ov9282, OV9282_REG_MODE_SELECT, 1, OV9282_MODE_STANDBY);
if (ret)
return ret;

/* Low power mode */
ret = ov9282_write_reg(ov9282, OV9282_REG_POWER_CTRL, 1, 0x01);
if (ret)
return ret;

/* External trigger snapshot */
ret = ov9282_write_reg(ov9282, OV9282_REG_LOW_POWER_MODE_CTRL, 1, 0x04);
if (ret)
return ret;

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

ret = ov9282_write_reg(ov9282, OV9282_REG_SLEEP_PERIOD_CTRL0, 1, 0x00);
if (ret)
return ret;

ret = ov9282_write_reg(ov9282, OV9282_REG_SLEEP_PERIOD_CTRL3, 1, 0x7F);
if (ret)
return ret;

/* No auto wake */
ret = ov9282_write_reg(ov9282, OV9282_REG_TIMING_23, 1, 0x00);
if (ret)
return ret;

/* stay standby mode and wait for trigger signal */
ret = ov9282_write_reg(ov9282, OV9282_REG_MODE_SELECT, 1, OV9282_MODE_STANDBY);
if (ret)
return ret;

return 0;
}

/**
* ov9282_start_streaming() - Start sensor stream
* @ov9282: pointer to ov9282 device
Expand All @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

int bitdepth_index;
int ret;

/* Write common registers */
ret = ov9282_write_regs(ov9282, common_regs_list.regs,
common_regs_list.num_of_regs);
ret = ov9282_write_regs(ov9282, common_regs_list.regs, common_regs_list.num_of_regs);
if (ret) {
dev_err(ov9282->dev, "fail to write common registers");
return ret;
Expand All @@ -992,15 +1050,24 @@ 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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whitespace change

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

if (ret) {
dev_err(ov9282->dev, "fail to setup handler");
return ret;
}

/* 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;
}
return 0;
}

/* 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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if (ret) {
dev_err(ov9282->dev, "fail to start streaming");
return ret;
Expand Down Expand Up @@ -1392,6 +1459,7 @@ static int ov9282_probe(struct i2c_client *client)
{
struct ov9282 *ov9282;
int ret;
u32 trig_mod;

ov9282 = devm_kzalloc(&client->dev, sizeof(*ov9282), GFP_KERNEL);
if (!ov9282)
Expand Down Expand Up @@ -1431,6 +1499,9 @@ static int ov9282_probe(struct i2c_client *client)
ov9282->code = MEDIA_BUS_FMT_Y10_1X10;
ov9282->vblank = ov9282->cur_mode->vblank;

ret = of_property_read_u32(client->dev.of_node, "trigger-mode", &trig_mod);
ov9282->trigger_mode = (ret == 0) ? trig_mod : -1;

ret = ov9282_init_controls(ov9282);
if (ret) {
dev_err(ov9282->dev, "failed to init controls: %d", ret);
Expand Down