Hello all
This series extends the ov8865 driver with:
* Support for binding to ACPI enumerated devices.
* Support for a 19.2MHz clock in addition to existing 24MHz clock support
* Another v4l2_subdev_pad_ops callback
* 4 more V4L2 controls
* makes the driver supported by the cio2-bridge
There's also a little bit of tidying that I did along the way.
The series is tested on an MS Surface Go 2.
Thanks
Dan
Daniel Scally (13):
media: i2c: Add ACPI support to ov8865
media: i2c: Fix incorrect value in comment
media: i2c: Defer probe if not endpoint found
media: i2c: Support 19.2MHz input clock in ov8865
media: i2c: Add .get_selection() support to ov8865
media: i2c: Switch control to V4L2_CID_ANALOGUE_GAIN
media: i2c: Add vblank control to ov8865
media: i2c: Add hblank control to ov8865
media: i2c: cap exposure at height + vblank in ov8865
media: i2c: Add controls from fwnode to ov8865
media: i2c: Switch exposure control unit to lines
media: i2c: Remove unused macros from ov8865
media: ipu3-cio2: Add INT347A to cio2-bridge
drivers/media/i2c/ov8865.c | 465 ++++++++++++---------
drivers/media/pci/intel/ipu3/cio2-bridge.c | 2 +
2 files changed, 277 insertions(+), 190 deletions(-)
--
2.25.1
The PLL configuration defined here sets 72MHz (which is correct), not
80MHz. Correct the comment.
Signed-off-by: Daniel Scally <[email protected]>
---
drivers/media/i2c/ov8865.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c
index fe60cda3dea7..2ef146e7e7ef 100644
--- a/drivers/media/i2c/ov8865.c
+++ b/drivers/media/i2c/ov8865.c
@@ -713,7 +713,7 @@ static const struct ov8865_pll2_config ov8865_pll2_config_native = {
/*
* EXTCLK = 24 MHz
* DAC_CLK = 360 MHz
- * SCLK = 80 MHz
+ * SCLK = 72 MHz
*/
static const struct ov8865_pll2_config ov8865_pll2_config_binning = {
--
2.25.1
The ov8865 driver's v4l2_subdev_pad_ops currently does not include
.get_selection() - add support for that callback.
Signed-off-by: Daniel Scally <[email protected]>
---
drivers/media/i2c/ov8865.c | 61 ++++++++++++++++++++++++++++++++++++++
1 file changed, 61 insertions(+)
diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c
index 8739eea762c5..c012f5cb11ab 100644
--- a/drivers/media/i2c/ov8865.c
+++ b/drivers/media/i2c/ov8865.c
@@ -450,6 +450,15 @@
#define OV8865_PRE_CTRL0_PATTERN_COLOR_SQUARES 2
#define OV8865_PRE_CTRL0_PATTERN_BLACK 3
+/* Pixel Array */
+
+#define OV8865_NATIVE_WIDTH 3296
+#define OV8865_NATIVE_HEIGHT 2528
+#define OV8865_ACTIVE_START_TOP 32
+#define OV8865_ACTIVE_START_LEFT 80
+#define OV8865_ACTIVE_WIDTH 3264
+#define OV8865_ACTIVE_HEIGHT 2448
+
/* Macros */
#define ov8865_subdev_sensor(s) \
@@ -2743,12 +2752,64 @@ static int ov8865_enum_frame_interval(struct v4l2_subdev *subdev,
return 0;
}
+static void
+__ov8865_get_pad_crop(struct ov8865_sensor *sensor,
+ struct v4l2_subdev_state *state, unsigned int pad,
+ enum v4l2_subdev_format_whence which, struct v4l2_rect *r)
+{
+ switch (which) {
+ case V4L2_SUBDEV_FORMAT_TRY:
+ *r = *v4l2_subdev_get_try_crop(&sensor->subdev, state, pad);
+ break;
+ case V4L2_SUBDEV_FORMAT_ACTIVE:
+ r->height = sensor->state.mode->output_size_y;
+ r->width = sensor->state.mode->output_size_x;
+ r->top = (OV8865_NATIVE_HEIGHT - sensor->state.mode->output_size_y) / 2;
+ r->left = (OV8865_NATIVE_WIDTH - sensor->state.mode->output_size_x) / 2;
+ break;
+ }
+}
+
+static int ov8865_get_selection(struct v4l2_subdev *subdev,
+ struct v4l2_subdev_state *state,
+ struct v4l2_subdev_selection *sel)
+{
+ struct ov8865_sensor *sensor = ov8865_subdev_sensor(subdev);
+
+ switch (sel->target) {
+ case V4L2_SEL_TGT_CROP:
+ mutex_lock(&sensor->mutex);
+ __ov8865_get_pad_crop(sensor, state, sel->pad,
+ sel->which, &sel->r);
+ mutex_unlock(&sensor->mutex);
+ break;
+ case V4L2_SEL_TGT_NATIVE_SIZE:
+ sel->r.top = 0;
+ sel->r.left = 0;
+ sel->r.width = OV8865_NATIVE_WIDTH;
+ sel->r.height = OV8865_NATIVE_HEIGHT;
+ break;
+ case V4L2_SEL_TGT_CROP_BOUNDS:
+ case V4L2_SEL_TGT_CROP_DEFAULT:
+ sel->r.top = OV8865_ACTIVE_START_TOP;
+ sel->r.left = OV8865_ACTIVE_START_LEFT;
+ sel->r.width = OV8865_ACTIVE_WIDTH;
+ sel->r.height = OV8865_ACTIVE_HEIGHT;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static const struct v4l2_subdev_pad_ops ov8865_subdev_pad_ops = {
.enum_mbus_code = ov8865_enum_mbus_code,
.get_fmt = ov8865_get_fmt,
.set_fmt = ov8865_set_fmt,
.enum_frame_size = ov8865_enum_frame_size,
.enum_frame_interval = ov8865_enum_frame_interval,
+ .get_selection = ov8865_get_selection,
};
static const struct v4l2_subdev_ops ov8865_subdev_ops = {
--
2.25.1
Add a V4L2_CID_VBLANK control to the ov8865 driver.
Signed-off-by: Daniel Scally <[email protected]>
---
drivers/media/i2c/ov8865.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c
index 09558a3342dd..daead1fc9314 100644
--- a/drivers/media/i2c/ov8865.c
+++ b/drivers/media/i2c/ov8865.c
@@ -183,6 +183,8 @@
#define OV8865_VTS_H(v) (((v) & GENMASK(11, 8)) >> 8)
#define OV8865_VTS_L_REG 0x380f
#define OV8865_VTS_L(v) ((v) & GENMASK(7, 0))
+#define OV8865_TIMING_MAX_VTS 0xffff
+#define OV8865_TIMING_MIN_VTS 0x04
#define OV8865_OFFSET_X_H_REG 0x3810
#define OV8865_OFFSET_X_H(v) (((v) & GENMASK(15, 8)) >> 8)
#define OV8865_OFFSET_X_L_REG 0x3811
@@ -658,6 +660,7 @@ struct ov8865_state {
struct ov8865_ctrls {
struct v4l2_ctrl *link_freq;
struct v4l2_ctrl *pixel_rate;
+ struct v4l2_ctrl *vblank;
struct v4l2_ctrl_handler handler;
};
@@ -2212,6 +2215,20 @@ static int ov8865_test_pattern_configure(struct ov8865_sensor *sensor,
ov8865_test_pattern_bits[index]);
}
+/* Blanking */
+
+static int ov8865_vts_configure(struct ov8865_sensor *sensor, u32 vblank)
+{
+ u16 vts = sensor->state.mode->output_size_y + vblank;
+ int ret;
+
+ ret = ov8865_write(sensor, OV8865_VTS_H_REG, OV8865_VTS_H(vts));
+ if (ret)
+ return ret;
+
+ return ov8865_write(sensor, OV8865_VTS_L_REG, OV8865_VTS_L(vts));
+}
+
/* State */
static int ov8865_state_mipi_configure(struct ov8865_sensor *sensor,
@@ -2463,6 +2480,8 @@ static int ov8865_s_ctrl(struct v4l2_ctrl *ctrl)
case V4L2_CID_TEST_PATTERN:
index = (unsigned int)ctrl->val;
return ov8865_test_pattern_configure(sensor, index);
+ case V4L2_CID_VBLANK:
+ return ov8865_vts_configure(sensor, ctrl->val);
default:
return -EINVAL;
}
@@ -2479,6 +2498,8 @@ static int ov8865_ctrls_init(struct ov8865_sensor *sensor)
struct ov8865_ctrls *ctrls = &sensor->ctrls;
struct v4l2_ctrl_handler *handler = &ctrls->handler;
const struct v4l2_ctrl_ops *ops = &ov8865_ctrl_ops;
+ const struct ov8865_mode *mode = sensor->state.mode;
+ unsigned int vblank_max, vblank_def;
int ret;
v4l2_ctrl_handler_init(handler, 32);
@@ -2514,6 +2535,13 @@ static int ov8865_ctrls_init(struct ov8865_sensor *sensor)
ARRAY_SIZE(ov8865_test_pattern_menu) - 1,
0, 0, ov8865_test_pattern_menu);
+ /* Blanking */
+ vblank_max = OV8865_TIMING_MAX_VTS - mode->output_size_y;
+ vblank_def = mode->vts - mode->output_size_y;
+ ctrls->vblank = v4l2_ctrl_new_std(handler, ops, V4L2_CID_VBLANK,
+ OV8865_TIMING_MIN_VTS, vblank_max, 1,
+ vblank_def);
+
/* MIPI CSI-2 */
ctrls->link_freq =
@@ -2694,6 +2722,10 @@ static int ov8865_set_fmt(struct v4l2_subdev *subdev,
sensor->state.mbus_code != mbus_code)
ret = ov8865_state_configure(sensor, mode, mbus_code);
+ __v4l2_ctrl_modify_range(sensor->ctrls.vblank, OV8865_TIMING_MIN_VTS,
+ OV8865_TIMING_MAX_VTS - mode->output_size_y,
+ 1, mode->vts - mode->output_size_y);
+
complete:
mutex_unlock(&sensor->mutex);
@@ -3019,6 +3051,8 @@ static int ov8865_probe(struct i2c_client *client)
/* Sensor */
+ sensor->state.mode = &ov8865_modes[0];
+
ret = ov8865_ctrls_init(sensor);
if (ret)
goto error_mutex;
--
2.25.1
The V4L2_CID_GAIN control for this driver configures registers that
the datasheet specifies as analogue gain. Switch the control's ID
to V4L2_CID_ANALOGUE_GAIN.
Signed-off-by: Daniel Scally <[email protected]>
---
drivers/media/i2c/ov8865.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c
index c012f5cb11ab..09558a3342dd 100644
--- a/drivers/media/i2c/ov8865.c
+++ b/drivers/media/i2c/ov8865.c
@@ -2137,7 +2137,7 @@ static int ov8865_exposure_configure(struct ov8865_sensor *sensor, u32 exposure)
/* Gain */
-static int ov8865_gain_configure(struct ov8865_sensor *sensor, u32 gain)
+static int ov8865_analog_gain_configure(struct ov8865_sensor *sensor, u32 gain)
{
int ret;
@@ -2447,8 +2447,8 @@ static int ov8865_s_ctrl(struct v4l2_ctrl *ctrl)
if (ret)
return ret;
break;
- case V4L2_CID_GAIN:
- ret = ov8865_gain_configure(sensor, ctrl->val);
+ case V4L2_CID_ANALOGUE_GAIN:
+ ret = ov8865_analog_gain_configure(sensor, ctrl->val);
if (ret)
return ret;
break;
@@ -2493,7 +2493,7 @@ static int ov8865_ctrls_init(struct ov8865_sensor *sensor)
/* Gain */
- v4l2_ctrl_new_std(handler, ops, V4L2_CID_GAIN, 128, 8191, 128, 128);
+ v4l2_ctrl_new_std(handler, ops, V4L2_CID_ANALOGUE_GAIN, 128, 8191, 128, 128);
/* White Balance */
--
2.25.1
Add V4L2_CID_CAMERA_ORIENTATION and V4L2_CID_CAMERA_SENSOR_ROTATION
controls to the ov8865 driver by attempting to parse them from firmware.
Signed-off-by: Daniel Scally <[email protected]>
---
drivers/media/i2c/ov8865.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c
index 941b0f94f249..4954808f5416 100644
--- a/drivers/media/i2c/ov8865.c
+++ b/drivers/media/i2c/ov8865.c
@@ -2513,6 +2513,7 @@ static int ov8865_ctrls_init(struct ov8865_sensor *sensor)
struct v4l2_ctrl_handler *handler = &ctrls->handler;
const struct v4l2_ctrl_ops *ops = &ov8865_ctrl_ops;
const struct ov8865_mode *mode = sensor->state.mode;
+ struct v4l2_fwnode_device_properties props;
unsigned int vblank_max, vblank_def;
unsigned int hblank;
int ret;
@@ -2575,6 +2576,15 @@ static int ov8865_ctrls_init(struct ov8865_sensor *sensor)
v4l2_ctrl_new_std(handler, NULL, V4L2_CID_PIXEL_RATE, 1,
INT_MAX, 1, 1);
+ /* set properties from fwnode (e.g. rotation, orientation) */
+ ret = v4l2_fwnode_device_parse(sensor->dev, &props);
+ if (ret)
+ goto error_ctrls;
+
+ ret = v4l2_ctrl_new_fwnode_properties(handler, ops, &props);
+ if (ret)
+ goto error_ctrls;
+
if (handler->error) {
ret = handler->error;
goto error_ctrls;
--
2.25.1
The ov8865 driver currently has the unit of the V4L2_CID_EXPOSURE control
as 1/16th of a line. This is what the sensor expects, but isn't very
intuitive. Switch the control to be in units of a line and simply do the
16x multiplication before passing the value to the sensor.
The datasheet for this sensor gives minimum exposure as 2 lines, so take
the opportunity to correct the lower bounds of the control.
Signed-off-by: Daniel Scally <[email protected]>
---
drivers/media/i2c/ov8865.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c
index 4954808f5416..dca4db3039bb 100644
--- a/drivers/media/i2c/ov8865.c
+++ b/drivers/media/i2c/ov8865.c
@@ -2126,6 +2126,9 @@ static int ov8865_exposure_configure(struct ov8865_sensor *sensor, u32 exposure)
{
int ret;
+ /* The sensor stores exposure in units of 1/16th of a line */
+ exposure *= 16;
+
ret = ov8865_write(sensor, OV8865_EXPOSURE_CTRL_HH_REG,
OV8865_EXPOSURE_CTRL_HH(exposure));
if (ret)
@@ -2525,8 +2528,8 @@ static int ov8865_ctrls_init(struct ov8865_sensor *sensor)
/* Exposure */
- ctrls->exposure = v4l2_ctrl_new_std(handler, ops, V4L2_CID_EXPOSURE, 16,
- 1048575, 16, 512);
+ ctrls->exposure = v4l2_ctrl_new_std(handler, ops, V4L2_CID_EXPOSURE, 2,
+ 65535, 1, 32);
/* Gain */
--
2.25.1
The ov8865 driver is one of those that can be connected to a CIO2
device by the cio2-bridge code. This means that the absence of an
endpoint for this device is not necessarily fatal, as one might be
built by the cio2-bridge when it probes. Return -EPROBE_DEFER if no
endpoint is found rather than a fatal error.
Signed-off-by: Daniel Scally <[email protected]>
---
drivers/media/i2c/ov8865.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c
index 2ef146e7e7ef..66182142c28b 100644
--- a/drivers/media/i2c/ov8865.c
+++ b/drivers/media/i2c/ov8865.c
@@ -2796,10 +2796,8 @@ static int ov8865_probe(struct i2c_client *client)
/* Graph Endpoint */
handle = fwnode_graph_get_next_endpoint(dev_fwnode(dev), NULL);
- if (!handle) {
- dev_err(dev, "unable to find endpoint node\n");
- return -EINVAL;
- }
+ if (!handle)
+ return -EPROBE_DEFER;
sensor->endpoint.bus_type = V4L2_MBUS_CSI2_DPHY;
--
2.25.1
Add a V4L2_CID_HBLANK control to the ov8865 driver. This is read only
with timing control intended to be done via vblanking alone.
Signed-off-by: Daniel Scally <[email protected]>
---
One of the modes defined in this driver actually has HTS as a _lower_ value
than output_size_x, which means the usual hblank = (hts - output_size_y) formula
doesn't make sense. That seems really strange to me, but the Windows driver
does it too so my understanding must be lacking there...I handled that by
flooring hblank at 0, but there may be a much better way of doing this.
drivers/media/i2c/ov8865.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c
index daead1fc9314..e1d3c0d50fdc 100644
--- a/drivers/media/i2c/ov8865.c
+++ b/drivers/media/i2c/ov8865.c
@@ -660,6 +660,7 @@ struct ov8865_state {
struct ov8865_ctrls {
struct v4l2_ctrl *link_freq;
struct v4l2_ctrl *pixel_rate;
+ struct v4l2_ctrl *hblank;
struct v4l2_ctrl *vblank;
struct v4l2_ctrl_handler handler;
@@ -2500,6 +2501,7 @@ static int ov8865_ctrls_init(struct ov8865_sensor *sensor)
const struct v4l2_ctrl_ops *ops = &ov8865_ctrl_ops;
const struct ov8865_mode *mode = sensor->state.mode;
unsigned int vblank_max, vblank_def;
+ unsigned int hblank;
int ret;
v4l2_ctrl_handler_init(handler, 32);
@@ -2536,6 +2538,13 @@ static int ov8865_ctrls_init(struct ov8865_sensor *sensor)
0, 0, ov8865_test_pattern_menu);
/* Blanking */
+ hblank = mode->hts < mode->output_size_x ? 0 : mode->hts - mode->output_size_x;
+ ctrls->hblank = v4l2_ctrl_new_std(handler, ops, V4L2_CID_HBLANK, hblank,
+ hblank, 1, hblank);
+
+ if (ctrls->hblank)
+ ctrls->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+
vblank_max = OV8865_TIMING_MAX_VTS - mode->output_size_y;
vblank_def = mode->vts - mode->output_size_y;
ctrls->vblank = v4l2_ctrl_new_std(handler, ops, V4L2_CID_VBLANK,
@@ -2682,6 +2691,7 @@ static int ov8865_set_fmt(struct v4l2_subdev *subdev,
struct v4l2_mbus_framefmt *mbus_format = &format->format;
const struct ov8865_mode *mode;
u32 mbus_code = 0;
+ unsigned int hblank;
unsigned int index;
int ret = 0;
@@ -2726,6 +2736,10 @@ static int ov8865_set_fmt(struct v4l2_subdev *subdev,
OV8865_TIMING_MAX_VTS - mode->output_size_y,
1, mode->vts - mode->output_size_y);
+ hblank = mode->hts < mode->output_size_x ? 0 : mode->hts - mode->output_size_x;
+ __v4l2_ctrl_modify_range(sensor->ctrls.hblank, hblank, hblank, 1,
+ hblank);
+
complete:
mutex_unlock(&sensor->mutex);
--
2.25.1
The ov8865 driver as written expects a 24MHz input clock, but the sensor
is sometimes found on x86 platforms with a 19.2MHz input clock supplied.
Add a set of PLL configurations to the driver to support that rate too.
As ACPI doesn't auto-configure the clock rate, check for a clock-frequency
during probe and set that rate if one is found.
Signed-off-by: Daniel Scally <[email protected]>
---
drivers/media/i2c/ov8865.c | 158 +++++++++++++++++++++++++++----------
1 file changed, 115 insertions(+), 43 deletions(-)
diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c
index 66182142c28b..8739eea762c5 100644
--- a/drivers/media/i2c/ov8865.c
+++ b/drivers/media/i2c/ov8865.c
@@ -21,10 +21,6 @@
#include <media/v4l2-image-sizes.h>
#include <media/v4l2-mediabus.h>
-/* Clock rate */
-
-#define OV8865_EXTCLK_RATE 24000000
-
/* Register definitions */
/* System */
@@ -665,6 +661,9 @@ struct ov8865_sensor {
struct regulator *avdd;
struct regulator *dvdd;
struct regulator *dovdd;
+
+ unsigned long extclk_rate;
+ unsigned int extclk_rate_idx;
struct clk *extclk;
struct v4l2_fwnode_endpoint endpoint;
@@ -680,49 +679,83 @@ struct ov8865_sensor {
/* Static definitions */
/*
- * EXTCLK = 24 MHz
* PHY_SCLK = 720 MHz
* MIPI_PCLK = 90 MHz
*/
-static const struct ov8865_pll1_config ov8865_pll1_config_native = {
- .pll_pre_div_half = 1,
- .pll_pre_div = 0,
- .pll_mul = 30,
- .m_div = 1,
- .mipi_div = 3,
- .pclk_div = 1,
- .sys_pre_div = 1,
- .sys_div = 2,
+
+static const struct ov8865_pll1_config ov8865_pll1_configs_native[] = {
+ { /* 19.2 MHz input clock */
+ .pll_pre_div_half = 1,
+ .pll_pre_div = 2,
+ .pll_mul = 75,
+ .m_div = 1,
+ .mipi_div = 3,
+ .pclk_div = 1,
+ .sys_pre_div = 1,
+ .sys_div = 2,
+ },
+ { /* 24MHz input clock */
+ .pll_pre_div_half = 1,
+ .pll_pre_div = 0,
+ .pll_mul = 30,
+ .m_div = 1,
+ .mipi_div = 3,
+ .pclk_div = 1,
+ .sys_pre_div = 1,
+ .sys_div = 2,
+ },
};
/*
- * EXTCLK = 24 MHz
* DAC_CLK = 360 MHz
* SCLK = 144 MHz
*/
-static const struct ov8865_pll2_config ov8865_pll2_config_native = {
- .pll_pre_div_half = 1,
- .pll_pre_div = 0,
- .pll_mul = 30,
- .dac_div = 2,
- .sys_pre_div = 5,
- .sys_div = 0,
+static const struct ov8865_pll2_config ov8865_pll2_configs_native[] = {
+ /* 19.2MHz input clock */
+ {
+ .pll_pre_div_half = 1,
+ .pll_pre_div = 5,
+ .pll_mul = 75,
+ .dac_div = 1,
+ .sys_pre_div = 1,
+ .sys_div = 3,
+ },
+ /* 24MHz input clock */
+ {
+ .pll_pre_div_half = 1,
+ .pll_pre_div = 0,
+ .pll_mul = 30,
+ .dac_div = 2,
+ .sys_pre_div = 5,
+ .sys_div = 0,
+ }
};
/*
- * EXTCLK = 24 MHz
* DAC_CLK = 360 MHz
* SCLK = 72 MHz
*/
-static const struct ov8865_pll2_config ov8865_pll2_config_binning = {
+static const struct ov8865_pll2_config ov8865_pll2_configs_binning[] = {
+ /* 19.2MHz input clock */
+ {
+ .pll_pre_div_half = 1,
+ .pll_pre_div = 2,
+ .pll_mul = 75,
+ .dac_div = 2,
+ .sys_pre_div = 10,
+ .sys_div = 0,
+ },
+ /* 24MHz input clock */
+ {
.pll_pre_div_half = 1,
.pll_pre_div = 0,
.pll_mul = 30,
.dac_div = 2,
.sys_pre_div = 10,
.sys_div = 0,
+ }
};
static const struct ov8865_sclk_config ov8865_sclk_config_native = {
@@ -934,8 +967,8 @@ static const struct ov8865_mode ov8865_modes[] = {
.frame_interval = { 1, 30 },
/* PLL */
- .pll1_config = &ov8865_pll1_config_native,
- .pll2_config = &ov8865_pll2_config_native,
+ .pll1_config = ov8865_pll1_configs_native,
+ .pll2_config = ov8865_pll2_configs_native,
.sclk_config = &ov8865_sclk_config_native,
/* Registers */
@@ -990,8 +1023,8 @@ static const struct ov8865_mode ov8865_modes[] = {
.frame_interval = { 1, 30 },
/* PLL */
- .pll1_config = &ov8865_pll1_config_native,
- .pll2_config = &ov8865_pll2_config_native,
+ .pll1_config = ov8865_pll1_configs_native,
+ .pll2_config = ov8865_pll2_configs_native,
.sclk_config = &ov8865_sclk_config_native,
/* Registers */
@@ -1050,8 +1083,8 @@ static const struct ov8865_mode ov8865_modes[] = {
.frame_interval = { 1, 30 },
/* PLL */
- .pll1_config = &ov8865_pll1_config_native,
- .pll2_config = &ov8865_pll2_config_binning,
+ .pll1_config = ov8865_pll1_configs_native,
+ .pll2_config = ov8865_pll2_configs_binning,
.sclk_config = &ov8865_sclk_config_native,
/* Registers */
@@ -1116,8 +1149,8 @@ static const struct ov8865_mode ov8865_modes[] = {
.frame_interval = { 1, 90 },
/* PLL */
- .pll1_config = &ov8865_pll1_config_native,
- .pll2_config = &ov8865_pll2_config_binning,
+ .pll1_config = ov8865_pll1_configs_native,
+ .pll2_config = ov8865_pll2_configs_binning,
.sclk_config = &ov8865_sclk_config_native,
/* Registers */
@@ -1266,6 +1299,13 @@ static const struct ov8865_register_value ov8865_init_sequence[] = {
{ 0x4503, 0x10 },
};
+/* Clock rate */
+
+static const unsigned long supported_extclk_rates[] = {
+ 19200000,
+ 24000000,
+};
+
static const s64 ov8865_link_freq_menu[] = {
360000000,
};
@@ -1513,12 +1553,11 @@ static int ov8865_isp_configure(struct ov8865_sensor *sensor)
static unsigned long ov8865_mode_pll1_rate(struct ov8865_sensor *sensor,
const struct ov8865_mode *mode)
{
- const struct ov8865_pll1_config *config = mode->pll1_config;
- unsigned long extclk_rate;
+ const struct ov8865_pll1_config *config;
unsigned long pll1_rate;
- extclk_rate = clk_get_rate(sensor->extclk);
- pll1_rate = extclk_rate * config->pll_mul / config->pll_pre_div_half;
+ config = &mode->pll1_config[sensor->extclk_rate_idx];
+ pll1_rate = sensor->extclk_rate * config->pll_mul / config->pll_pre_div_half;
switch (config->pll_pre_div) {
case 0:
@@ -1552,10 +1591,12 @@ static int ov8865_mode_pll1_configure(struct ov8865_sensor *sensor,
const struct ov8865_mode *mode,
u32 mbus_code)
{
- const struct ov8865_pll1_config *config = mode->pll1_config;
+ const struct ov8865_pll1_config *config;
u8 value;
int ret;
+ config = &mode->pll1_config[sensor->extclk_rate_idx];
+
switch (mbus_code) {
case MEDIA_BUS_FMT_SBGGR10_1X10:
value = OV8865_MIPI_BIT_SEL(10);
@@ -1622,9 +1663,11 @@ static int ov8865_mode_pll1_configure(struct ov8865_sensor *sensor,
static int ov8865_mode_pll2_configure(struct ov8865_sensor *sensor,
const struct ov8865_mode *mode)
{
- const struct ov8865_pll2_config *config = mode->pll2_config;
+ const struct ov8865_pll2_config *config;
int ret;
+ config = &mode->pll2_config[sensor->extclk_rate_idx];
+
ret = ov8865_write(sensor, OV8865_PLL_CTRL12_REG,
OV8865_PLL_CTRL12_PRE_DIV_HALF(config->pll_pre_div_half) |
OV8865_PLL_CTRL12_DAC_DIV(config->dac_div));
@@ -2053,9 +2096,11 @@ static int ov8865_mode_configure(struct ov8865_sensor *sensor,
static unsigned long ov8865_mode_mipi_clk_rate(struct ov8865_sensor *sensor,
const struct ov8865_mode *mode)
{
- const struct ov8865_pll1_config *config = mode->pll1_config;
+ const struct ov8865_pll1_config *config;
unsigned long pll1_rate;
+ config = &mode->pll1_config[sensor->extclk_rate_idx];
+
pll1_rate = ov8865_mode_pll1_rate(sensor, mode);
return pll1_rate / config->m_div / 2;
@@ -2783,7 +2828,8 @@ static int ov8865_probe(struct i2c_client *client)
struct ov8865_sensor *sensor;
struct v4l2_subdev *subdev;
struct media_pad *pad;
- unsigned long rate;
+ unsigned int rate;
+ unsigned int i;
int ret;
sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
@@ -2858,13 +2904,39 @@ static int ov8865_probe(struct i2c_client *client)
goto error_endpoint;
}
- rate = clk_get_rate(sensor->extclk);
- if (rate != OV8865_EXTCLK_RATE) {
- dev_err(dev, "clock rate %lu Hz is unsupported\n", rate);
+ /*
+ * We could have either a 24MHz or 19.2MHz clock rate. Check for a
+ * clock-frequency property and if found, set that rate. This should
+ * cover ACPI case. If the system uses devicetree then the configured
+ * rate should already be set, so we'll have to check it.
+ */
+
+ ret = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency",
+ &rate);
+ if (!ret) {
+ ret = clk_set_rate(sensor->extclk, rate);
+ if (ret) {
+ dev_err(dev, "failed to set clock rate\n");
+ return ret;
+ }
+ }
+
+ sensor->extclk_rate = clk_get_rate(sensor->extclk);
+
+ for (i = 0; i < ARRAY_SIZE(supported_extclk_rates); i++) {
+ if (sensor->extclk_rate == supported_extclk_rates[i])
+ break;
+ }
+
+ if (i == ARRAY_SIZE(supported_extclk_rates)) {
+ dev_err(dev, "clock rate %lu Hz is unsupported\n",
+ sensor->extclk_rate);
ret = -EINVAL;
goto error_endpoint;
}
+ sensor->extclk_rate_idx = i;
+
/* Subdev, entity and pad */
subdev = &sensor->subdev;
--
2.25.1
Exposure limits depend on the total height; when vblank is altered (and
thus the total height is altered), change the exposure limits to reflect
the new cap.
Signed-off-by: Daniel Scally <[email protected]>
---
drivers/media/i2c/ov8865.c | 24 ++++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c
index e1d3c0d50fdc..941b0f94f249 100644
--- a/drivers/media/i2c/ov8865.c
+++ b/drivers/media/i2c/ov8865.c
@@ -662,6 +662,7 @@ struct ov8865_ctrls {
struct v4l2_ctrl *pixel_rate;
struct v4l2_ctrl *hblank;
struct v4l2_ctrl *vblank;
+ struct v4l2_ctrl *exposure;
struct v4l2_ctrl_handler handler;
};
@@ -2455,6 +2456,18 @@ static int ov8865_s_ctrl(struct v4l2_ctrl *ctrl)
unsigned int index;
int ret;
+ /* If VBLANK is altered we need to update exposure to compensate */
+ if (ctrl->id == V4L2_CID_VBLANK) {
+ int exposure_max;
+
+ exposure_max = sensor->state.mode->output_size_y + ctrl->val;
+ __v4l2_ctrl_modify_range(sensor->ctrls.exposure,
+ sensor->ctrls.exposure->minimum,
+ exposure_max,
+ sensor->ctrls.exposure->step,
+ min(sensor->ctrls.exposure->val, exposure_max));
+ }
+
/* Wait for the sensor to be on before setting controls. */
if (pm_runtime_suspended(sensor->dev))
return 0;
@@ -2511,8 +2524,8 @@ static int ov8865_ctrls_init(struct ov8865_sensor *sensor)
/* Exposure */
- v4l2_ctrl_new_std(handler, ops, V4L2_CID_EXPOSURE, 16, 1048575, 16,
- 512);
+ ctrls->exposure = v4l2_ctrl_new_std(handler, ops, V4L2_CID_EXPOSURE, 16,
+ 1048575, 16, 512);
/* Gain */
@@ -2693,6 +2706,7 @@ static int ov8865_set_fmt(struct v4l2_subdev *subdev,
u32 mbus_code = 0;
unsigned int hblank;
unsigned int index;
+ int exposure_max;
int ret = 0;
mutex_lock(&sensor->mutex);
@@ -2740,6 +2754,12 @@ static int ov8865_set_fmt(struct v4l2_subdev *subdev,
__v4l2_ctrl_modify_range(sensor->ctrls.hblank, hblank, hblank, 1,
hblank);
+ exposure_max = mode->vts;
+ __v4l2_ctrl_modify_range(sensor->ctrls.exposure,
+ sensor->ctrls.exposure->minimum, exposure_max,
+ sensor->ctrls.exposure->step,
+ min(sensor->ctrls.exposure->val, exposure_max));
+
complete:
mutex_unlock(&sensor->mutex);
--
2.25.1
There are a number of macros defined in this driver that aren't actually
used within it. There's a lot of macros defined in total, so removing the
unused ones helps make it a bit less busy.
Signed-off-by: Daniel Scally <[email protected]>
---
I wavered about including this, because it might be helpful for someone adding
support for other features in the future to have these already defined, but in
the end I thought it slightly better to be less busy.
drivers/media/i2c/ov8865.c | 137 +------------------------------------
1 file changed, 1 insertion(+), 136 deletions(-)
diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c
index dca4db3039bb..9b38f2e16906 100644
--- a/drivers/media/i2c/ov8865.c
+++ b/drivers/media/i2c/ov8865.c
@@ -46,8 +46,6 @@
#define OV8865_PLL_CTRL6_REG 0x306
#define OV8865_PLL_CTRL6_SYS_DIV(v) (((v) - 1) & BIT(0))
-#define OV8865_PLL_CTRL8_REG 0x308
-#define OV8865_PLL_CTRL9_REG 0x309
#define OV8865_PLL_CTRLA_REG 0x30a
#define OV8865_PLL_CTRLA_PRE_DIV_HALF(v) (((v) - 1) & BIT(0))
#define OV8865_PLL_CTRLB_REG 0x30b
@@ -60,41 +58,21 @@
#define OV8865_PLL_CTRLE_SYS_DIV(v) ((v) & GENMASK(2, 0))
#define OV8865_PLL_CTRLF_REG 0x30f
#define OV8865_PLL_CTRLF_SYS_PRE_DIV(v) (((v) - 1) & GENMASK(3, 0))
-#define OV8865_PLL_CTRL10_REG 0x310
-#define OV8865_PLL_CTRL11_REG 0x311
#define OV8865_PLL_CTRL12_REG 0x312
#define OV8865_PLL_CTRL12_PRE_DIV_HALF(v) ((((v) - 1) << 4) & BIT(4))
#define OV8865_PLL_CTRL12_DAC_DIV(v) (((v) - 1) & GENMASK(3, 0))
-#define OV8865_PLL_CTRL1B_REG 0x31b
-#define OV8865_PLL_CTRL1C_REG 0x31c
-
#define OV8865_PLL_CTRL1E_REG 0x31e
#define OV8865_PLL_CTRL1E_PLL1_NO_LAT BIT(3)
-#define OV8865_PAD_OEN0_REG 0x3000
-
-#define OV8865_PAD_OEN2_REG 0x3002
-
-#define OV8865_CLK_RST5_REG 0x3005
-
#define OV8865_CHIP_ID_HH_REG 0x300a
#define OV8865_CHIP_ID_HH_VALUE 0x00
#define OV8865_CHIP_ID_H_REG 0x300b
#define OV8865_CHIP_ID_H_VALUE 0x88
#define OV8865_CHIP_ID_L_REG 0x300c
#define OV8865_CHIP_ID_L_VALUE 0x65
-#define OV8865_PAD_OUT2_REG 0x300d
-
-#define OV8865_PAD_SEL2_REG 0x3010
-#define OV8865_PAD_PK_REG 0x3011
-#define OV8865_PAD_PK_DRIVE_STRENGTH_1X (0 << 5)
-#define OV8865_PAD_PK_DRIVE_STRENGTH_2X (1 << 5)
-#define OV8865_PAD_PK_DRIVE_STRENGTH_3X (2 << 5)
-#define OV8865_PAD_PK_DRIVE_STRENGTH_4X (3 << 5)
#define OV8865_PUMP_CLK_DIV_REG 0x3015
-#define OV8865_PUMP_CLK_DIV_PUMP_N(v) (((v) << 4) & GENMASK(6, 4))
#define OV8865_PUMP_CLK_DIV_PUMP_P(v) ((v) & GENMASK(2, 0))
#define OV8865_MIPI_SC_CTRL0_REG 0x3018
@@ -102,21 +80,12 @@
GENMASK(7, 5))
#define OV8865_MIPI_SC_CTRL0_MIPI_EN BIT(4)
#define OV8865_MIPI_SC_CTRL0_UNKNOWN BIT(1)
-#define OV8865_MIPI_SC_CTRL0_LANES_PD_MIPI BIT(0)
-#define OV8865_MIPI_SC_CTRL1_REG 0x3019
-#define OV8865_CLK_RST0_REG 0x301a
-#define OV8865_CLK_RST1_REG 0x301b
-#define OV8865_CLK_RST2_REG 0x301c
-#define OV8865_CLK_RST3_REG 0x301d
-#define OV8865_CLK_RST4_REG 0x301e
#define OV8865_PCLK_SEL_REG 0x3020
#define OV8865_PCLK_SEL_PCLK_DIV_MASK BIT(3)
#define OV8865_PCLK_SEL_PCLK_DIV(v) ((((v) - 1) << 3) & BIT(3))
-#define OV8865_MISC_CTRL_REG 0x3021
#define OV8865_MIPI_SC_CTRL2_REG 0x3022
-#define OV8865_MIPI_SC_CTRL2_CLK_LANES_PD_MIPI BIT(1)
#define OV8865_MIPI_SC_CTRL2_PD_MIPI_RST_SYNC BIT(0)
#define OV8865_MIPI_BIT_SEL_REG 0x3031
@@ -125,7 +94,6 @@
#define OV8865_CLK_SEL0_PLL1_SYS_SEL(v) (((v) << 7) & BIT(7))
#define OV8865_CLK_SEL1_REG 0x3033
#define OV8865_CLK_SEL1_MIPI_EOF BIT(5)
-#define OV8865_CLK_SEL1_UNKNOWN BIT(2)
#define OV8865_CLK_SEL1_PLL_SCLK_SEL_MASK BIT(1)
#define OV8865_CLK_SEL1_PLL_SCLK_SEL(v) (((v) << 1) & BIT(1))
@@ -142,7 +110,6 @@
#define OV8865_EXPOSURE_CTRL_H(v) (((v) & GENMASK(15, 8)) >> 8)
#define OV8865_EXPOSURE_CTRL_L_REG 0x3502
#define OV8865_EXPOSURE_CTRL_L(v) ((v) & GENMASK(7, 0))
-#define OV8865_EXPOSURE_GAIN_MANUAL_REG 0x3503
#define OV8865_GAIN_CTRL_H_REG 0x3508
#define OV8865_GAIN_CTRL_H(v) (((v) & GENMASK(12, 8)) >> 8)
@@ -197,18 +164,6 @@
#define OV8865_INC_X_ODD(v) ((v) & GENMASK(4, 0))
#define OV8865_INC_X_EVEN_REG 0x3815
#define OV8865_INC_X_EVEN(v) ((v) & GENMASK(4, 0))
-#define OV8865_VSYNC_START_H_REG 0x3816
-#define OV8865_VSYNC_START_H(v) (((v) & GENMASK(15, 8)) >> 8)
-#define OV8865_VSYNC_START_L_REG 0x3817
-#define OV8865_VSYNC_START_L(v) ((v) & GENMASK(7, 0))
-#define OV8865_VSYNC_END_H_REG 0x3818
-#define OV8865_VSYNC_END_H(v) (((v) & GENMASK(15, 8)) >> 8)
-#define OV8865_VSYNC_END_L_REG 0x3819
-#define OV8865_VSYNC_END_L(v) ((v) & GENMASK(7, 0))
-#define OV8865_HSYNC_FIRST_H_REG 0x381a
-#define OV8865_HSYNC_FIRST_H(v) (((v) & GENMASK(15, 8)) >> 8)
-#define OV8865_HSYNC_FIRST_L_REG 0x381b
-#define OV8865_HSYNC_FIRST_L(v) ((v) & GENMASK(7, 0))
#define OV8865_FORMAT1_REG 0x3820
#define OV8865_FORMAT1_FLIP_VERT_ISP_EN BIT(2)
@@ -240,10 +195,6 @@
#define OV8865_AUTO_SIZE_CTRL_CROP_END_X_REG BIT(2)
#define OV8865_AUTO_SIZE_CTRL_CROP_START_Y_REG BIT(1)
#define OV8865_AUTO_SIZE_CTRL_CROP_START_X_REG BIT(0)
-#define OV8865_AUTO_SIZE_X_OFFSET_H_REG 0x3842
-#define OV8865_AUTO_SIZE_X_OFFSET_L_REG 0x3843
-#define OV8865_AUTO_SIZE_Y_OFFSET_H_REG 0x3844
-#define OV8865_AUTO_SIZE_Y_OFFSET_L_REG 0x3845
#define OV8865_AUTO_SIZE_BOUNDARIES_REG 0x3846
#define OV8865_AUTO_SIZE_BOUNDARIES_Y(v) (((v) << 4) & GENMASK(7, 4))
#define OV8865_AUTO_SIZE_BOUNDARIES_X(v) ((v) & GENMASK(3, 0))
@@ -259,30 +210,10 @@
#define OV8865_BLC_CTRL0_TRIG_FORMAT_EN BIT(6)
#define OV8865_BLC_CTRL0_TRIG_GAIN_EN BIT(5)
#define OV8865_BLC_CTRL0_TRIG_EXPOSURE_EN BIT(4)
-#define OV8865_BLC_CTRL0_TRIG_MANUAL_EN BIT(3)
-#define OV8865_BLC_CTRL0_FREEZE_EN BIT(2)
-#define OV8865_BLC_CTRL0_ALWAYS_EN BIT(1)
#define OV8865_BLC_CTRL0_FILTER_EN BIT(0)
#define OV8865_BLC_CTRL1_REG 0x4001
-#define OV8865_BLC_CTRL1_DITHER_EN BIT(7)
-#define OV8865_BLC_CTRL1_ZERO_LINE_DIFF_EN BIT(6)
-#define OV8865_BLC_CTRL1_COL_SHIFT_256 (0 << 4)
#define OV8865_BLC_CTRL1_COL_SHIFT_128 (1 << 4)
-#define OV8865_BLC_CTRL1_COL_SHIFT_64 (2 << 4)
-#define OV8865_BLC_CTRL1_COL_SHIFT_32 (3 << 4)
#define OV8865_BLC_CTRL1_OFFSET_LIMIT_EN BIT(2)
-#define OV8865_BLC_CTRL1_COLUMN_CANCEL_EN BIT(1)
-#define OV8865_BLC_CTRL2_REG 0x4002
-#define OV8865_BLC_CTRL3_REG 0x4003
-#define OV8865_BLC_CTRL4_REG 0x4004
-#define OV8865_BLC_CTRL5_REG 0x4005
-#define OV8865_BLC_CTRL6_REG 0x4006
-#define OV8865_BLC_CTRL7_REG 0x4007
-#define OV8865_BLC_CTRL8_REG 0x4008
-#define OV8865_BLC_CTRL9_REG 0x4009
-#define OV8865_BLC_CTRLA_REG 0x400a
-#define OV8865_BLC_CTRLB_REG 0x400b
-#define OV8865_BLC_CTRLC_REG 0x400c
#define OV8865_BLC_CTRLD_REG 0x400d
#define OV8865_BLC_CTRLD_OFFSET_TRIGGER(v) ((v) & GENMASK(7, 0))
@@ -337,66 +268,8 @@
/* MIPI */
-#define OV8865_MIPI_CTRL0_REG 0x4800
-#define OV8865_MIPI_CTRL1_REG 0x4801
-#define OV8865_MIPI_CTRL2_REG 0x4802
-#define OV8865_MIPI_CTRL3_REG 0x4803
-#define OV8865_MIPI_CTRL4_REG 0x4804
-#define OV8865_MIPI_CTRL5_REG 0x4805
-#define OV8865_MIPI_CTRL6_REG 0x4806
-#define OV8865_MIPI_CTRL7_REG 0x4807
-#define OV8865_MIPI_CTRL8_REG 0x4808
-
-#define OV8865_MIPI_FCNT_MAX_H_REG 0x4810
-#define OV8865_MIPI_FCNT_MAX_L_REG 0x4811
-
-#define OV8865_MIPI_CTRL13_REG 0x4813
-#define OV8865_MIPI_CTRL14_REG 0x4814
-#define OV8865_MIPI_CTRL15_REG 0x4815
-#define OV8865_MIPI_EMBEDDED_DT_REG 0x4816
-
-#define OV8865_MIPI_HS_ZERO_MIN_H_REG 0x4818
-#define OV8865_MIPI_HS_ZERO_MIN_L_REG 0x4819
-#define OV8865_MIPI_HS_TRAIL_MIN_H_REG 0x481a
-#define OV8865_MIPI_HS_TRAIL_MIN_L_REG 0x481b
-#define OV8865_MIPI_CLK_ZERO_MIN_H_REG 0x481c
-#define OV8865_MIPI_CLK_ZERO_MIN_L_REG 0x481d
-#define OV8865_MIPI_CLK_PREPARE_MAX_REG 0x481e
-#define OV8865_MIPI_CLK_PREPARE_MIN_REG 0x481f
-#define OV8865_MIPI_CLK_POST_MIN_H_REG 0x4820
-#define OV8865_MIPI_CLK_POST_MIN_L_REG 0x4821
-#define OV8865_MIPI_CLK_TRAIL_MIN_H_REG 0x4822
-#define OV8865_MIPI_CLK_TRAIL_MIN_L_REG 0x4823
-#define OV8865_MIPI_LPX_P_MIN_H_REG 0x4824
-#define OV8865_MIPI_LPX_P_MIN_L_REG 0x4825
-#define OV8865_MIPI_HS_PREPARE_MIN_REG 0x4826
-#define OV8865_MIPI_HS_PREPARE_MAX_REG 0x4827
-#define OV8865_MIPI_HS_EXIT_MIN_H_REG 0x4828
-#define OV8865_MIPI_HS_EXIT_MIN_L_REG 0x4829
-#define OV8865_MIPI_UI_HS_ZERO_MIN_REG 0x482a
-#define OV8865_MIPI_UI_HS_TRAIL_MIN_REG 0x482b
-#define OV8865_MIPI_UI_CLK_ZERO_MIN_REG 0x482c
-#define OV8865_MIPI_UI_CLK_PREPARE_REG 0x482d
-#define OV8865_MIPI_UI_CLK_POST_MIN_REG 0x482e
-#define OV8865_MIPI_UI_CLK_TRAIL_MIN_REG 0x482f
-#define OV8865_MIPI_UI_LPX_P_MIN_REG 0x4830
-#define OV8865_MIPI_UI_HS_PREPARE_REG 0x4831
-#define OV8865_MIPI_UI_HS_EXIT_MIN_REG 0x4832
-#define OV8865_MIPI_PKT_START_SIZE_REG 0x4833
-
#define OV8865_MIPI_PCLK_PERIOD_REG 0x4837
-#define OV8865_MIPI_LP_GPIO0_REG 0x4838
-#define OV8865_MIPI_LP_GPIO1_REG 0x4839
-
-#define OV8865_MIPI_CTRL3C_REG 0x483c
-#define OV8865_MIPI_LP_GPIO4_REG 0x483d
-
-#define OV8865_MIPI_CTRL4A_REG 0x484a
-#define OV8865_MIPI_CTRL4B_REG 0x484b
-#define OV8865_MIPI_CTRL4C_REG 0x484c
-#define OV8865_MIPI_LANE_TEST_PATTERN_REG 0x484d
-#define OV8865_MIPI_FRAME_END_DELAY_REG 0x484e
-#define OV8865_MIPI_CLOCK_TEST_PATTERN_REG 0x484f
+
#define OV8865_MIPI_LANE_SEL01_REG 0x4850
#define OV8865_MIPI_LANE_SEL01_LANE0(v) (((v) << 0) & GENMASK(2, 0))
#define OV8865_MIPI_LANE_SEL01_LANE1(v) (((v) << 4) & GENMASK(6, 4))
@@ -407,7 +280,6 @@
/* ISP */
#define OV8865_ISP_CTRL0_REG 0x5000
-#define OV8865_ISP_CTRL0_LENC_EN BIT(7)
#define OV8865_ISP_CTRL0_WHITE_BALANCE_EN BIT(4)
#define OV8865_ISP_CTRL0_DPC_BLACK_EN BIT(2)
#define OV8865_ISP_CTRL0_DPC_WHITE_EN BIT(1)
@@ -416,17 +288,11 @@
#define OV8865_ISP_CTRL2_REG 0x5002
#define OV8865_ISP_CTRL2_DEBUG BIT(3)
#define OV8865_ISP_CTRL2_VARIOPIXEL_EN BIT(2)
-#define OV8865_ISP_CTRL2_VSYNC_LATCH_EN BIT(0)
-#define OV8865_ISP_CTRL3_REG 0x5003
#define OV8865_ISP_GAIN_RED_H_REG 0x5018
#define OV8865_ISP_GAIN_RED_H(v) (((v) & GENMASK(13, 6)) >> 6)
#define OV8865_ISP_GAIN_RED_L_REG 0x5019
#define OV8865_ISP_GAIN_RED_L(v) ((v) & GENMASK(5, 0))
-#define OV8865_ISP_GAIN_GREEN_H_REG 0x501a
-#define OV8865_ISP_GAIN_GREEN_H(v) (((v) & GENMASK(13, 6)) >> 6)
-#define OV8865_ISP_GAIN_GREEN_L_REG 0x501b
-#define OV8865_ISP_GAIN_GREEN_L(v) ((v) & GENMASK(5, 0))
#define OV8865_ISP_GAIN_BLUE_H_REG 0x501c
#define OV8865_ISP_GAIN_BLUE_H(v) (((v) & GENMASK(13, 6)) >> 6)
#define OV8865_ISP_GAIN_BLUE_L_REG 0x501d
@@ -434,7 +300,6 @@
/* VarioPixel */
-#define OV8865_VAP_CTRL0_REG 0x5900
#define OV8865_VAP_CTRL1_REG 0x5901
#define OV8865_VAP_CTRL1_HSUB_COEF(v) ((((v) - 1) << 2) & \
GENMASK(3, 2))
--
2.25.1
ACPI _HID INT347A represents the OV8865 sensor, the driver for which can
support the platforms that the cio2-bridge serves. Add it to the array
of supported sensors so the bridge will connect the sensor to the CIO2
device.
Signed-off-by: Daniel Scally <[email protected]>
---
drivers/media/pci/intel/ipu3/cio2-bridge.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/media/pci/intel/ipu3/cio2-bridge.c b/drivers/media/pci/intel/ipu3/cio2-bridge.c
index 4657e99df033..6195abd7582c 100644
--- a/drivers/media/pci/intel/ipu3/cio2-bridge.c
+++ b/drivers/media/pci/intel/ipu3/cio2-bridge.c
@@ -24,6 +24,8 @@ static const struct cio2_sensor_config cio2_supported_sensors[] = {
CIO2_SENSOR_CONFIG("INT33BE", 0),
/* Omnivision OV2680 */
CIO2_SENSOR_CONFIG("OVTI2680", 0),
+ /* Omnivision OV8865 */
+ CIO2_SENSOR_CONFIG("INT347A", 1, 360000000),
};
static const struct cio2_property_names prop_names = {
--
2.25.1
Hi Andy
On 22/07/2021 23:22, Andy Shevchenko wrote:
>
>
> On Thursday, July 22, 2021, Daniel Scally <[email protected]
> <mailto:[email protected]>> wrote:
>
> ACPI _HID INT347A represents the OV8865 sensor, the driver for
> which can
> support the platforms that the cio2-bridge serves. Add it to the array
> of supported sensors so the bridge will connect the sensor to the CIO2
> device.
>
> Signed-off-by: Daniel Scally <[email protected]
> <mailto:[email protected]>>
> ---
> Â drivers/media/pci/intel/ipu3/cio2-bridge.c | 2 ++
> Â 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/media/pci/intel/ipu3/cio2-bridge.c
> b/drivers/media/pci/intel/ipu3/cio2-bridge.c
> index 4657e99df033..6195abd7582c 100644
> --- a/drivers/media/pci/intel/ipu3/cio2-bridge.c
> +++ b/drivers/media/pci/intel/ipu3/cio2-bridge.c
> @@ -24,6 +24,8 @@ static const struct cio2_sensor_config
> cio2_supported_sensors[] = {
> Â Â Â Â CIO2_SENSOR_CONFIG("INT33BE", 0),
> Â Â Â Â /* Omnivision OV2680 */
> Â Â Â Â CIO2_SENSOR_CONFIG("OVTI2680", 0),
> +Â Â Â Â /* Omnivision OV8865 */
> +Â Â Â Â CIO2_SENSOR_CONFIG("INT347A", 1, 360000000),
>
>
> I assume it may be positioned at any index in the array. I prefer to
> see them sorted by HID
Yeah any position's fine; I seem to struggle with alphabetisation for
some reason :) No problem, I'll order them properly in v2.
>
>
> Â
>
> Â };
>
> Â static const struct cio2_property_names prop_names = {
> --
> 2.25.1
>
>
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
Hi,
On Thu 22 Jul 21, 21:33, Daniel Scally wrote:
> The PLL configuration defined here sets 72MHz (which is correct), not
> 80MHz. Correct the comment.
This is:
Reviewed-by: Paul Kocialkowski <[email protected]>
Thanks,
Paul
> Signed-off-by: Daniel Scally <[email protected]>
> ---
> drivers/media/i2c/ov8865.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c
> index fe60cda3dea7..2ef146e7e7ef 100644
> --- a/drivers/media/i2c/ov8865.c
> +++ b/drivers/media/i2c/ov8865.c
> @@ -713,7 +713,7 @@ static const struct ov8865_pll2_config ov8865_pll2_config_native = {
> /*
> * EXTCLK = 24 MHz
> * DAC_CLK = 360 MHz
> - * SCLK = 80 MHz
> + * SCLK = 72 MHz
> */
>
> static const struct ov8865_pll2_config ov8865_pll2_config_binning = {
> --
> 2.25.1
>
--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
Hi Daniel,
On Thu 22 Jul 21, 21:33, Daniel Scally wrote:
> The ov8865 driver is one of those that can be connected to a CIO2
> device by the cio2-bridge code. This means that the absence of an
> endpoint for this device is not necessarily fatal, as one might be
> built by the cio2-bridge when it probes. Return -EPROBE_DEFER if no
> endpoint is found rather than a fatal error.
Is this an error that you have actually seen in practice?
My understanding is that this function should return the handle to the *local*
fwnode graph endpoint, which relates to the static device-tree description
and should be unrelated to another driver probing.
So as far as I can see, this should not be needed (but correct me if I'm wrong).
Cheers,
Paul
> Signed-off-by: Daniel Scally <[email protected]>
> ---
> drivers/media/i2c/ov8865.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c
> index 2ef146e7e7ef..66182142c28b 100644
> --- a/drivers/media/i2c/ov8865.c
> +++ b/drivers/media/i2c/ov8865.c
> @@ -2796,10 +2796,8 @@ static int ov8865_probe(struct i2c_client *client)
> /* Graph Endpoint */
>
> handle = fwnode_graph_get_next_endpoint(dev_fwnode(dev), NULL);
> - if (!handle) {
> - dev_err(dev, "unable to find endpoint node\n");
> - return -EINVAL;
> - }
> + if (!handle)
> + return -EPROBE_DEFER;
>
> sensor->endpoint.bus_type = V4L2_MBUS_CSI2_DPHY;
>
> --
> 2.25.1
>
--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
Hi,
On Thu 22 Jul 21, 21:34, Daniel Scally wrote:
> The V4L2_CID_GAIN control for this driver configures registers that
> the datasheet specifies as analogue gain. Switch the control's ID
> to V4L2_CID_ANALOGUE_GAIN.
I had some doubts about this when writing the driver because it's called
"AEC gain" but it seems that you're right. The datasheet also defines
0x350a and 0x350b as digital gain (which are unused by the driver).
This is:
Reviewed-by: Paul Kocialkowski <[email protected]>
Cheers,
Paul
> Signed-off-by: Daniel Scally <[email protected]>
> ---
> drivers/media/i2c/ov8865.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c
> index c012f5cb11ab..09558a3342dd 100644
> --- a/drivers/media/i2c/ov8865.c
> +++ b/drivers/media/i2c/ov8865.c
> @@ -2137,7 +2137,7 @@ static int ov8865_exposure_configure(struct ov8865_sensor *sensor, u32 exposure)
>
> /* Gain */
>
> -static int ov8865_gain_configure(struct ov8865_sensor *sensor, u32 gain)
> +static int ov8865_analog_gain_configure(struct ov8865_sensor *sensor, u32 gain)
> {
> int ret;
>
> @@ -2447,8 +2447,8 @@ static int ov8865_s_ctrl(struct v4l2_ctrl *ctrl)
> if (ret)
> return ret;
> break;
> - case V4L2_CID_GAIN:
> - ret = ov8865_gain_configure(sensor, ctrl->val);
> + case V4L2_CID_ANALOGUE_GAIN:
> + ret = ov8865_analog_gain_configure(sensor, ctrl->val);
> if (ret)
> return ret;
> break;
> @@ -2493,7 +2493,7 @@ static int ov8865_ctrls_init(struct ov8865_sensor *sensor)
>
> /* Gain */
>
> - v4l2_ctrl_new_std(handler, ops, V4L2_CID_GAIN, 128, 8191, 128, 128);
> + v4l2_ctrl_new_std(handler, ops, V4L2_CID_ANALOGUE_GAIN, 128, 8191, 128, 128);
>
> /* White Balance */
>
> --
> 2.25.1
>
--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
Hi,
On Fri 23 Jul 21, 01:19, Andy Shevchenko wrote:
> On Thursday, July 22, 2021, Daniel Scally <[email protected]> wrote:
>
> > There are a number of macros defined in this driver that aren't actually
> > used within it. There's a lot of macros defined in total, so removing the
> > unused ones helps make it a bit less busy.
> >
> > Signed-off-by: Daniel Scally <[email protected]>
> > ---
> >
> > I wavered about including this, because it might be helpful for someone
> > adding
> > support for other features in the future to have these already defined,
> > but in
> > the end I thought it slightly better to be less busy.
>
> Exactly! I would leave the registers and bitfield definitions untouched as
> they play role of documentation. Of course even if you remove them, they
> will be in the history, but a) harder to access; b) adding new feature may
> introduce slightly different names for the same things.
I agree that it's better to keep them around. for the same reasons.
Cheers,
Paul
> > drivers/media/i2c/ov8865.c | 137 +------------------------------------
> > 1 file changed, 1 insertion(+), 136 deletions(-)
> >
> > diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c
> > index dca4db3039bb..9b38f2e16906 100644
> > --- a/drivers/media/i2c/ov8865.c
> > +++ b/drivers/media/i2c/ov8865.c
> > @@ -46,8 +46,6 @@
> > #define OV8865_PLL_CTRL6_REG 0x306
> > #define OV8865_PLL_CTRL6_SYS_DIV(v) (((v) - 1) & BIT(0))
> >
> > -#define OV8865_PLL_CTRL8_REG 0x308
> > -#define OV8865_PLL_CTRL9_REG 0x309
> > #define OV8865_PLL_CTRLA_REG 0x30a
> > #define OV8865_PLL_CTRLA_PRE_DIV_HALF(v) (((v) - 1) & BIT(0))
> > #define OV8865_PLL_CTRLB_REG 0x30b
> > @@ -60,41 +58,21 @@
> > #define OV8865_PLL_CTRLE_SYS_DIV(v) ((v) & GENMASK(2, 0))
> > #define OV8865_PLL_CTRLF_REG 0x30f
> > #define OV8865_PLL_CTRLF_SYS_PRE_DIV(v) (((v) - 1) &
> > GENMASK(3, 0))
> > -#define OV8865_PLL_CTRL10_REG 0x310
> > -#define OV8865_PLL_CTRL11_REG 0x311
> > #define OV8865_PLL_CTRL12_REG 0x312
> > #define OV8865_PLL_CTRL12_PRE_DIV_HALF(v) ((((v) - 1) << 4) &
> > BIT(4))
> > #define OV8865_PLL_CTRL12_DAC_DIV(v) (((v) - 1) & GENMASK(3, 0))
> >
> > -#define OV8865_PLL_CTRL1B_REG 0x31b
> > -#define OV8865_PLL_CTRL1C_REG 0x31c
> > -
> > #define OV8865_PLL_CTRL1E_REG 0x31e
> > #define OV8865_PLL_CTRL1E_PLL1_NO_LAT BIT(3)
> >
> > -#define OV8865_PAD_OEN0_REG 0x3000
> > -
> > -#define OV8865_PAD_OEN2_REG 0x3002
> > -
> > -#define OV8865_CLK_RST5_REG 0x3005
> > -
> > #define OV8865_CHIP_ID_HH_REG 0x300a
> > #define OV8865_CHIP_ID_HH_VALUE 0x00
> > #define OV8865_CHIP_ID_H_REG 0x300b
> > #define OV8865_CHIP_ID_H_VALUE 0x88
> > #define OV8865_CHIP_ID_L_REG 0x300c
> > #define OV8865_CHIP_ID_L_VALUE 0x65
> > -#define OV8865_PAD_OUT2_REG 0x300d
> > -
> > -#define OV8865_PAD_SEL2_REG 0x3010
> > -#define OV8865_PAD_PK_REG 0x3011
> > -#define OV8865_PAD_PK_DRIVE_STRENGTH_1X (0 << 5)
> > -#define OV8865_PAD_PK_DRIVE_STRENGTH_2X (1 << 5)
> > -#define OV8865_PAD_PK_DRIVE_STRENGTH_3X (2 << 5)
> > -#define OV8865_PAD_PK_DRIVE_STRENGTH_4X (3 << 5)
> >
> > #define OV8865_PUMP_CLK_DIV_REG 0x3015
> > -#define OV8865_PUMP_CLK_DIV_PUMP_N(v) (((v) << 4) & GENMASK(6,
> > 4))
> > #define OV8865_PUMP_CLK_DIV_PUMP_P(v) ((v) & GENMASK(2, 0))
> >
> > #define OV8865_MIPI_SC_CTRL0_REG 0x3018
> > @@ -102,21 +80,12 @@
> > GENMASK(7, 5))
> > #define OV8865_MIPI_SC_CTRL0_MIPI_EN BIT(4)
> > #define OV8865_MIPI_SC_CTRL0_UNKNOWN BIT(1)
> > -#define OV8865_MIPI_SC_CTRL0_LANES_PD_MIPI BIT(0)
> > -#define OV8865_MIPI_SC_CTRL1_REG 0x3019
> > -#define OV8865_CLK_RST0_REG 0x301a
> > -#define OV8865_CLK_RST1_REG 0x301b
> > -#define OV8865_CLK_RST2_REG 0x301c
> > -#define OV8865_CLK_RST3_REG 0x301d
> > -#define OV8865_CLK_RST4_REG 0x301e
> >
> > #define OV8865_PCLK_SEL_REG 0x3020
> > #define OV8865_PCLK_SEL_PCLK_DIV_MASK BIT(3)
> > #define OV8865_PCLK_SEL_PCLK_DIV(v) ((((v) - 1) << 3) & BIT(3))
> >
> > -#define OV8865_MISC_CTRL_REG 0x3021
> > #define OV8865_MIPI_SC_CTRL2_REG 0x3022
> > -#define OV8865_MIPI_SC_CTRL2_CLK_LANES_PD_MIPI BIT(1)
> > #define OV8865_MIPI_SC_CTRL2_PD_MIPI_RST_SYNC BIT(0)
> >
> > #define OV8865_MIPI_BIT_SEL_REG 0x3031
> > @@ -125,7 +94,6 @@
> > #define OV8865_CLK_SEL0_PLL1_SYS_SEL(v) (((v) << 7) &
> > BIT(7))
> > #define OV8865_CLK_SEL1_REG 0x3033
> > #define OV8865_CLK_SEL1_MIPI_EOF BIT(5)
> > -#define OV8865_CLK_SEL1_UNKNOWN BIT(2)
> > #define OV8865_CLK_SEL1_PLL_SCLK_SEL_MASK BIT(1)
> > #define OV8865_CLK_SEL1_PLL_SCLK_SEL(v) (((v) << 1) &
> > BIT(1))
> >
> > @@ -142,7 +110,6 @@
> > #define OV8865_EXPOSURE_CTRL_H(v) (((v) & GENMASK(15, 8)) >>
> > 8)
> > #define OV8865_EXPOSURE_CTRL_L_REG 0x3502
> > #define OV8865_EXPOSURE_CTRL_L(v) ((v) & GENMASK(7, 0))
> > -#define OV8865_EXPOSURE_GAIN_MANUAL_REG 0x3503
> >
> > #define OV8865_GAIN_CTRL_H_REG 0x3508
> > #define OV8865_GAIN_CTRL_H(v) (((v) & GENMASK(12, 8)) >>
> > 8)
> > @@ -197,18 +164,6 @@
> > #define OV8865_INC_X_ODD(v) ((v) & GENMASK(4, 0))
> > #define OV8865_INC_X_EVEN_REG 0x3815
> > #define OV8865_INC_X_EVEN(v) ((v) & GENMASK(4, 0))
> > -#define OV8865_VSYNC_START_H_REG 0x3816
> > -#define OV8865_VSYNC_START_H(v) (((v) &
> > GENMASK(15, 8)) >> 8)
> > -#define OV8865_VSYNC_START_L_REG 0x3817
> > -#define OV8865_VSYNC_START_L(v) ((v) & GENMASK(7,
> > 0))
> > -#define OV8865_VSYNC_END_H_REG 0x3818
> > -#define OV8865_VSYNC_END_H(v) (((v) & GENMASK(15, 8)) >>
> > 8)
> > -#define OV8865_VSYNC_END_L_REG 0x3819
> > -#define OV8865_VSYNC_END_L(v) ((v) & GENMASK(7, 0))
> > -#define OV8865_HSYNC_FIRST_H_REG 0x381a
> > -#define OV8865_HSYNC_FIRST_H(v) (((v) &
> > GENMASK(15, 8)) >> 8)
> > -#define OV8865_HSYNC_FIRST_L_REG 0x381b
> > -#define OV8865_HSYNC_FIRST_L(v) ((v) & GENMASK(7,
> > 0))
> >
> > #define OV8865_FORMAT1_REG 0x3820
> > #define OV8865_FORMAT1_FLIP_VERT_ISP_EN BIT(2)
> > @@ -240,10 +195,6 @@
> > #define OV8865_AUTO_SIZE_CTRL_CROP_END_X_REG BIT(2)
> > #define OV8865_AUTO_SIZE_CTRL_CROP_START_Y_REG BIT(1)
> > #define OV8865_AUTO_SIZE_CTRL_CROP_START_X_REG BIT(0)
> > -#define OV8865_AUTO_SIZE_X_OFFSET_H_REG 0x3842
> > -#define OV8865_AUTO_SIZE_X_OFFSET_L_REG 0x3843
> > -#define OV8865_AUTO_SIZE_Y_OFFSET_H_REG 0x3844
> > -#define OV8865_AUTO_SIZE_Y_OFFSET_L_REG 0x3845
> > #define OV8865_AUTO_SIZE_BOUNDARIES_REG 0x3846
> > #define OV8865_AUTO_SIZE_BOUNDARIES_Y(v) (((v) << 4) & GENMASK(7,
> > 4))
> > #define OV8865_AUTO_SIZE_BOUNDARIES_X(v) ((v) & GENMASK(3, 0))
> > @@ -259,30 +210,10 @@
> > #define OV8865_BLC_CTRL0_TRIG_FORMAT_EN BIT(6)
> > #define OV8865_BLC_CTRL0_TRIG_GAIN_EN BIT(5)
> > #define OV8865_BLC_CTRL0_TRIG_EXPOSURE_EN BIT(4)
> > -#define OV8865_BLC_CTRL0_TRIG_MANUAL_EN BIT(3)
> > -#define OV8865_BLC_CTRL0_FREEZE_EN BIT(2)
> > -#define OV8865_BLC_CTRL0_ALWAYS_EN BIT(1)
> > #define OV8865_BLC_CTRL0_FILTER_EN BIT(0)
> > #define OV8865_BLC_CTRL1_REG 0x4001
> > -#define OV8865_BLC_CTRL1_DITHER_EN BIT(7)
> > -#define OV8865_BLC_CTRL1_ZERO_LINE_DIFF_EN BIT(6)
> > -#define OV8865_BLC_CTRL1_COL_SHIFT_256 (0 << 4)
> > #define OV8865_BLC_CTRL1_COL_SHIFT_128 (1 << 4)
> > -#define OV8865_BLC_CTRL1_COL_SHIFT_64 (2 << 4)
> > -#define OV8865_BLC_CTRL1_COL_SHIFT_32 (3 << 4)
> > #define OV8865_BLC_CTRL1_OFFSET_LIMIT_EN BIT(2)
> > -#define OV8865_BLC_CTRL1_COLUMN_CANCEL_EN BIT(1)
> > -#define OV8865_BLC_CTRL2_REG 0x4002
> > -#define OV8865_BLC_CTRL3_REG 0x4003
> > -#define OV8865_BLC_CTRL4_REG 0x4004
> > -#define OV8865_BLC_CTRL5_REG 0x4005
> > -#define OV8865_BLC_CTRL6_REG 0x4006
> > -#define OV8865_BLC_CTRL7_REG 0x4007
> > -#define OV8865_BLC_CTRL8_REG 0x4008
> > -#define OV8865_BLC_CTRL9_REG 0x4009
> > -#define OV8865_BLC_CTRLA_REG 0x400a
> > -#define OV8865_BLC_CTRLB_REG 0x400b
> > -#define OV8865_BLC_CTRLC_REG 0x400c
> > #define OV8865_BLC_CTRLD_REG 0x400d
> > #define OV8865_BLC_CTRLD_OFFSET_TRIGGER(v) ((v) & GENMASK(7, 0))
> >
> > @@ -337,66 +268,8 @@
> >
> > /* MIPI */
> >
> > -#define OV8865_MIPI_CTRL0_REG 0x4800
> > -#define OV8865_MIPI_CTRL1_REG 0x4801
> > -#define OV8865_MIPI_CTRL2_REG 0x4802
> > -#define OV8865_MIPI_CTRL3_REG 0x4803
> > -#define OV8865_MIPI_CTRL4_REG 0x4804
> > -#define OV8865_MIPI_CTRL5_REG 0x4805
> > -#define OV8865_MIPI_CTRL6_REG 0x4806
> > -#define OV8865_MIPI_CTRL7_REG 0x4807
> > -#define OV8865_MIPI_CTRL8_REG 0x4808
> > -
> > -#define OV8865_MIPI_FCNT_MAX_H_REG 0x4810
> > -#define OV8865_MIPI_FCNT_MAX_L_REG 0x4811
> > -
> > -#define OV8865_MIPI_CTRL13_REG 0x4813
> > -#define OV8865_MIPI_CTRL14_REG 0x4814
> > -#define OV8865_MIPI_CTRL15_REG 0x4815
> > -#define OV8865_MIPI_EMBEDDED_DT_REG 0x4816
> > -
> > -#define OV8865_MIPI_HS_ZERO_MIN_H_REG 0x4818
> > -#define OV8865_MIPI_HS_ZERO_MIN_L_REG 0x4819
> > -#define OV8865_MIPI_HS_TRAIL_MIN_H_REG 0x481a
> > -#define OV8865_MIPI_HS_TRAIL_MIN_L_REG 0x481b
> > -#define OV8865_MIPI_CLK_ZERO_MIN_H_REG 0x481c
> > -#define OV8865_MIPI_CLK_ZERO_MIN_L_REG 0x481d
> > -#define OV8865_MIPI_CLK_PREPARE_MAX_REG 0x481e
> > -#define OV8865_MIPI_CLK_PREPARE_MIN_REG 0x481f
> > -#define OV8865_MIPI_CLK_POST_MIN_H_REG 0x4820
> > -#define OV8865_MIPI_CLK_POST_MIN_L_REG 0x4821
> > -#define OV8865_MIPI_CLK_TRAIL_MIN_H_REG 0x4822
> > -#define OV8865_MIPI_CLK_TRAIL_MIN_L_REG 0x4823
> > -#define OV8865_MIPI_LPX_P_MIN_H_REG 0x4824
> > -#define OV8865_MIPI_LPX_P_MIN_L_REG 0x4825
> > -#define OV8865_MIPI_HS_PREPARE_MIN_REG 0x4826
> > -#define OV8865_MIPI_HS_PREPARE_MAX_REG 0x4827
> > -#define OV8865_MIPI_HS_EXIT_MIN_H_REG 0x4828
> > -#define OV8865_MIPI_HS_EXIT_MIN_L_REG 0x4829
> > -#define OV8865_MIPI_UI_HS_ZERO_MIN_REG 0x482a
> > -#define OV8865_MIPI_UI_HS_TRAIL_MIN_REG 0x482b
> > -#define OV8865_MIPI_UI_CLK_ZERO_MIN_REG 0x482c
> > -#define OV8865_MIPI_UI_CLK_PREPARE_REG 0x482d
> > -#define OV8865_MIPI_UI_CLK_POST_MIN_REG 0x482e
> > -#define OV8865_MIPI_UI_CLK_TRAIL_MIN_REG 0x482f
> > -#define OV8865_MIPI_UI_LPX_P_MIN_REG 0x4830
> > -#define OV8865_MIPI_UI_HS_PREPARE_REG 0x4831
> > -#define OV8865_MIPI_UI_HS_EXIT_MIN_REG 0x4832
> > -#define OV8865_MIPI_PKT_START_SIZE_REG 0x4833
> > -
> > #define OV8865_MIPI_PCLK_PERIOD_REG 0x4837
> > -#define OV8865_MIPI_LP_GPIO0_REG 0x4838
> > -#define OV8865_MIPI_LP_GPIO1_REG 0x4839
> > -
> > -#define OV8865_MIPI_CTRL3C_REG 0x483c
> > -#define OV8865_MIPI_LP_GPIO4_REG 0x483d
> > -
> > -#define OV8865_MIPI_CTRL4A_REG 0x484a
> > -#define OV8865_MIPI_CTRL4B_REG 0x484b
> > -#define OV8865_MIPI_CTRL4C_REG 0x484c
> > -#define OV8865_MIPI_LANE_TEST_PATTERN_REG 0x484d
> > -#define OV8865_MIPI_FRAME_END_DELAY_REG 0x484e
> > -#define OV8865_MIPI_CLOCK_TEST_PATTERN_REG 0x484f
> > +
> > #define OV8865_MIPI_LANE_SEL01_REG 0x4850
> > #define OV8865_MIPI_LANE_SEL01_LANE0(v) (((v) << 0) &
> > GENMASK(2, 0))
> > #define OV8865_MIPI_LANE_SEL01_LANE1(v) (((v) << 4) &
> > GENMASK(6, 4))
> > @@ -407,7 +280,6 @@
> > /* ISP */
> >
> > #define OV8865_ISP_CTRL0_REG 0x5000
> > -#define OV8865_ISP_CTRL0_LENC_EN BIT(7)
> > #define OV8865_ISP_CTRL0_WHITE_BALANCE_EN BIT(4)
> > #define OV8865_ISP_CTRL0_DPC_BLACK_EN BIT(2)
> > #define OV8865_ISP_CTRL0_DPC_WHITE_EN BIT(1)
> > @@ -416,17 +288,11 @@
> > #define OV8865_ISP_CTRL2_REG 0x5002
> > #define OV8865_ISP_CTRL2_DEBUG BIT(3)
> > #define OV8865_ISP_CTRL2_VARIOPIXEL_EN BIT(2)
> > -#define OV8865_ISP_CTRL2_VSYNC_LATCH_EN BIT(0)
> > -#define OV8865_ISP_CTRL3_REG 0x5003
> >
> > #define OV8865_ISP_GAIN_RED_H_REG 0x5018
> > #define OV8865_ISP_GAIN_RED_H(v) (((v) & GENMASK(13, 6)) >>
> > 6)
> > #define OV8865_ISP_GAIN_RED_L_REG 0x5019
> > #define OV8865_ISP_GAIN_RED_L(v) ((v) & GENMASK(5, 0))
> > -#define OV8865_ISP_GAIN_GREEN_H_REG 0x501a
> > -#define OV8865_ISP_GAIN_GREEN_H(v) (((v) & GENMASK(13, 6)) >>
> > 6)
> > -#define OV8865_ISP_GAIN_GREEN_L_REG 0x501b
> > -#define OV8865_ISP_GAIN_GREEN_L(v) ((v) & GENMASK(5, 0))
> > #define OV8865_ISP_GAIN_BLUE_H_REG 0x501c
> > #define OV8865_ISP_GAIN_BLUE_H(v) (((v) & GENMASK(13, 6)) >>
> > 6)
> > #define OV8865_ISP_GAIN_BLUE_L_REG 0x501d
> > @@ -434,7 +300,6 @@
> >
> > /* VarioPixel */
> >
> > -#define OV8865_VAP_CTRL0_REG 0x5900
> > #define OV8865_VAP_CTRL1_REG 0x5901
> > #define OV8865_VAP_CTRL1_HSUB_COEF(v) ((((v) - 1) << 2) & \
> > GENMASK(3, 2))
> > --
> > 2.25.1
> >
> >
>
> --
> With Best Regards,
> Andy Shevchenko
--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
Hi Daniel,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v5.14-rc2 next-20210722]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Daniel-Scally/Extensions-to-ov8865-driver/20210723-043624
base: git://linuxtv.org/media_tree.git master
config: i386-randconfig-m021-20210723 (attached as .config)
compiler: gcc-10 (Ubuntu 10.3.0-1ubuntu1~20.04) 10.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
smatch warnings:
drivers/media/i2c/ov8865.c:2784 ov8865_get_selection() warn: inconsistent indenting
vim +2784 drivers/media/i2c/ov8865.c
2772
2773 static int ov8865_get_selection(struct v4l2_subdev *subdev,
2774 struct v4l2_subdev_state *state,
2775 struct v4l2_subdev_selection *sel)
2776 {
2777 struct ov8865_sensor *sensor = ov8865_subdev_sensor(subdev);
2778
2779 switch (sel->target) {
2780 case V4L2_SEL_TGT_CROP:
2781 mutex_lock(&sensor->mutex);
2782 __ov8865_get_pad_crop(sensor, state, sel->pad,
2783 sel->which, &sel->r);
> 2784 mutex_unlock(&sensor->mutex);
2785 break;
2786 case V4L2_SEL_TGT_NATIVE_SIZE:
2787 sel->r.top = 0;
2788 sel->r.left = 0;
2789 sel->r.width = OV8865_NATIVE_WIDTH;
2790 sel->r.height = OV8865_NATIVE_HEIGHT;
2791 break;
2792 case V4L2_SEL_TGT_CROP_BOUNDS:
2793 case V4L2_SEL_TGT_CROP_DEFAULT:
2794 sel->r.top = OV8865_ACTIVE_START_TOP;
2795 sel->r.left = OV8865_ACTIVE_START_LEFT;
2796 sel->r.width = OV8865_ACTIVE_WIDTH;
2797 sel->r.height = OV8865_ACTIVE_HEIGHT;
2798 break;
2799 default:
2800 return -EINVAL;
2801 }
2802
2803 return 0;
2804 }
2805
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
On 23/07/2021 09:00, Paul Kocialkowski wrote:
> Hi,
>
> On Fri 23 Jul 21, 01:19, Andy Shevchenko wrote:
>> On Thursday, July 22, 2021, Daniel Scally <[email protected]> wrote:
>>
>>> There are a number of macros defined in this driver that aren't actually
>>> used within it. There's a lot of macros defined in total, so removing the
>>> unused ones helps make it a bit less busy.
>>>
>>> Signed-off-by: Daniel Scally <[email protected]>
>>> ---
>>>
>>> I wavered about including this, because it might be helpful for someone
>>> adding
>>> support for other features in the future to have these already defined,
>>> but in
>>> the end I thought it slightly better to be less busy.
>> Exactly! I would leave the registers and bitfield definitions untouched as
>> they play role of documentation. Of course even if you remove them, they
>> will be in the history, but a) harder to access; b) adding new feature may
>> introduce slightly different names for the same things.
> I agree that it's better to keep them around. for the same reasons.
>
> Cheers,
>
> Paul
Okedokey, will drop this one then - thanks both
>
>>> drivers/media/i2c/ov8865.c | 137 +------------------------------------
>>> 1 file changed, 1 insertion(+), 136 deletions(-)
>>>
>>> diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c
>>> index dca4db3039bb..9b38f2e16906 100644
>>> --- a/drivers/media/i2c/ov8865.c
>>> +++ b/drivers/media/i2c/ov8865.c
>>> @@ -46,8 +46,6 @@
>>> #define OV8865_PLL_CTRL6_REG 0x306
>>> #define OV8865_PLL_CTRL6_SYS_DIV(v) (((v) - 1) & BIT(0))
>>>
>>> -#define OV8865_PLL_CTRL8_REG 0x308
>>> -#define OV8865_PLL_CTRL9_REG 0x309
>>> #define OV8865_PLL_CTRLA_REG 0x30a
>>> #define OV8865_PLL_CTRLA_PRE_DIV_HALF(v) (((v) - 1) & BIT(0))
>>> #define OV8865_PLL_CTRLB_REG 0x30b
>>> @@ -60,41 +58,21 @@
>>> #define OV8865_PLL_CTRLE_SYS_DIV(v) ((v) & GENMASK(2, 0))
>>> #define OV8865_PLL_CTRLF_REG 0x30f
>>> #define OV8865_PLL_CTRLF_SYS_PRE_DIV(v) (((v) - 1) &
>>> GENMASK(3, 0))
>>> -#define OV8865_PLL_CTRL10_REG 0x310
>>> -#define OV8865_PLL_CTRL11_REG 0x311
>>> #define OV8865_PLL_CTRL12_REG 0x312
>>> #define OV8865_PLL_CTRL12_PRE_DIV_HALF(v) ((((v) - 1) << 4) &
>>> BIT(4))
>>> #define OV8865_PLL_CTRL12_DAC_DIV(v) (((v) - 1) & GENMASK(3, 0))
>>>
>>> -#define OV8865_PLL_CTRL1B_REG 0x31b
>>> -#define OV8865_PLL_CTRL1C_REG 0x31c
>>> -
>>> #define OV8865_PLL_CTRL1E_REG 0x31e
>>> #define OV8865_PLL_CTRL1E_PLL1_NO_LAT BIT(3)
>>>
>>> -#define OV8865_PAD_OEN0_REG 0x3000
>>> -
>>> -#define OV8865_PAD_OEN2_REG 0x3002
>>> -
>>> -#define OV8865_CLK_RST5_REG 0x3005
>>> -
>>> #define OV8865_CHIP_ID_HH_REG 0x300a
>>> #define OV8865_CHIP_ID_HH_VALUE 0x00
>>> #define OV8865_CHIP_ID_H_REG 0x300b
>>> #define OV8865_CHIP_ID_H_VALUE 0x88
>>> #define OV8865_CHIP_ID_L_REG 0x300c
>>> #define OV8865_CHIP_ID_L_VALUE 0x65
>>> -#define OV8865_PAD_OUT2_REG 0x300d
>>> -
>>> -#define OV8865_PAD_SEL2_REG 0x3010
>>> -#define OV8865_PAD_PK_REG 0x3011
>>> -#define OV8865_PAD_PK_DRIVE_STRENGTH_1X (0 << 5)
>>> -#define OV8865_PAD_PK_DRIVE_STRENGTH_2X (1 << 5)
>>> -#define OV8865_PAD_PK_DRIVE_STRENGTH_3X (2 << 5)
>>> -#define OV8865_PAD_PK_DRIVE_STRENGTH_4X (3 << 5)
>>>
>>> #define OV8865_PUMP_CLK_DIV_REG 0x3015
>>> -#define OV8865_PUMP_CLK_DIV_PUMP_N(v) (((v) << 4) & GENMASK(6,
>>> 4))
>>> #define OV8865_PUMP_CLK_DIV_PUMP_P(v) ((v) & GENMASK(2, 0))
>>>
>>> #define OV8865_MIPI_SC_CTRL0_REG 0x3018
>>> @@ -102,21 +80,12 @@
>>> GENMASK(7, 5))
>>> #define OV8865_MIPI_SC_CTRL0_MIPI_EN BIT(4)
>>> #define OV8865_MIPI_SC_CTRL0_UNKNOWN BIT(1)
>>> -#define OV8865_MIPI_SC_CTRL0_LANES_PD_MIPI BIT(0)
>>> -#define OV8865_MIPI_SC_CTRL1_REG 0x3019
>>> -#define OV8865_CLK_RST0_REG 0x301a
>>> -#define OV8865_CLK_RST1_REG 0x301b
>>> -#define OV8865_CLK_RST2_REG 0x301c
>>> -#define OV8865_CLK_RST3_REG 0x301d
>>> -#define OV8865_CLK_RST4_REG 0x301e
>>>
>>> #define OV8865_PCLK_SEL_REG 0x3020
>>> #define OV8865_PCLK_SEL_PCLK_DIV_MASK BIT(3)
>>> #define OV8865_PCLK_SEL_PCLK_DIV(v) ((((v) - 1) << 3) & BIT(3))
>>>
>>> -#define OV8865_MISC_CTRL_REG 0x3021
>>> #define OV8865_MIPI_SC_CTRL2_REG 0x3022
>>> -#define OV8865_MIPI_SC_CTRL2_CLK_LANES_PD_MIPI BIT(1)
>>> #define OV8865_MIPI_SC_CTRL2_PD_MIPI_RST_SYNC BIT(0)
>>>
>>> #define OV8865_MIPI_BIT_SEL_REG 0x3031
>>> @@ -125,7 +94,6 @@
>>> #define OV8865_CLK_SEL0_PLL1_SYS_SEL(v) (((v) << 7) &
>>> BIT(7))
>>> #define OV8865_CLK_SEL1_REG 0x3033
>>> #define OV8865_CLK_SEL1_MIPI_EOF BIT(5)
>>> -#define OV8865_CLK_SEL1_UNKNOWN BIT(2)
>>> #define OV8865_CLK_SEL1_PLL_SCLK_SEL_MASK BIT(1)
>>> #define OV8865_CLK_SEL1_PLL_SCLK_SEL(v) (((v) << 1) &
>>> BIT(1))
>>>
>>> @@ -142,7 +110,6 @@
>>> #define OV8865_EXPOSURE_CTRL_H(v) (((v) & GENMASK(15, 8)) >>
>>> 8)
>>> #define OV8865_EXPOSURE_CTRL_L_REG 0x3502
>>> #define OV8865_EXPOSURE_CTRL_L(v) ((v) & GENMASK(7, 0))
>>> -#define OV8865_EXPOSURE_GAIN_MANUAL_REG 0x3503
>>>
>>> #define OV8865_GAIN_CTRL_H_REG 0x3508
>>> #define OV8865_GAIN_CTRL_H(v) (((v) & GENMASK(12, 8)) >>
>>> 8)
>>> @@ -197,18 +164,6 @@
>>> #define OV8865_INC_X_ODD(v) ((v) & GENMASK(4, 0))
>>> #define OV8865_INC_X_EVEN_REG 0x3815
>>> #define OV8865_INC_X_EVEN(v) ((v) & GENMASK(4, 0))
>>> -#define OV8865_VSYNC_START_H_REG 0x3816
>>> -#define OV8865_VSYNC_START_H(v) (((v) &
>>> GENMASK(15, 8)) >> 8)
>>> -#define OV8865_VSYNC_START_L_REG 0x3817
>>> -#define OV8865_VSYNC_START_L(v) ((v) & GENMASK(7,
>>> 0))
>>> -#define OV8865_VSYNC_END_H_REG 0x3818
>>> -#define OV8865_VSYNC_END_H(v) (((v) & GENMASK(15, 8)) >>
>>> 8)
>>> -#define OV8865_VSYNC_END_L_REG 0x3819
>>> -#define OV8865_VSYNC_END_L(v) ((v) & GENMASK(7, 0))
>>> -#define OV8865_HSYNC_FIRST_H_REG 0x381a
>>> -#define OV8865_HSYNC_FIRST_H(v) (((v) &
>>> GENMASK(15, 8)) >> 8)
>>> -#define OV8865_HSYNC_FIRST_L_REG 0x381b
>>> -#define OV8865_HSYNC_FIRST_L(v) ((v) & GENMASK(7,
>>> 0))
>>>
>>> #define OV8865_FORMAT1_REG 0x3820
>>> #define OV8865_FORMAT1_FLIP_VERT_ISP_EN BIT(2)
>>> @@ -240,10 +195,6 @@
>>> #define OV8865_AUTO_SIZE_CTRL_CROP_END_X_REG BIT(2)
>>> #define OV8865_AUTO_SIZE_CTRL_CROP_START_Y_REG BIT(1)
>>> #define OV8865_AUTO_SIZE_CTRL_CROP_START_X_REG BIT(0)
>>> -#define OV8865_AUTO_SIZE_X_OFFSET_H_REG 0x3842
>>> -#define OV8865_AUTO_SIZE_X_OFFSET_L_REG 0x3843
>>> -#define OV8865_AUTO_SIZE_Y_OFFSET_H_REG 0x3844
>>> -#define OV8865_AUTO_SIZE_Y_OFFSET_L_REG 0x3845
>>> #define OV8865_AUTO_SIZE_BOUNDARIES_REG 0x3846
>>> #define OV8865_AUTO_SIZE_BOUNDARIES_Y(v) (((v) << 4) & GENMASK(7,
>>> 4))
>>> #define OV8865_AUTO_SIZE_BOUNDARIES_X(v) ((v) & GENMASK(3, 0))
>>> @@ -259,30 +210,10 @@
>>> #define OV8865_BLC_CTRL0_TRIG_FORMAT_EN BIT(6)
>>> #define OV8865_BLC_CTRL0_TRIG_GAIN_EN BIT(5)
>>> #define OV8865_BLC_CTRL0_TRIG_EXPOSURE_EN BIT(4)
>>> -#define OV8865_BLC_CTRL0_TRIG_MANUAL_EN BIT(3)
>>> -#define OV8865_BLC_CTRL0_FREEZE_EN BIT(2)
>>> -#define OV8865_BLC_CTRL0_ALWAYS_EN BIT(1)
>>> #define OV8865_BLC_CTRL0_FILTER_EN BIT(0)
>>> #define OV8865_BLC_CTRL1_REG 0x4001
>>> -#define OV8865_BLC_CTRL1_DITHER_EN BIT(7)
>>> -#define OV8865_BLC_CTRL1_ZERO_LINE_DIFF_EN BIT(6)
>>> -#define OV8865_BLC_CTRL1_COL_SHIFT_256 (0 << 4)
>>> #define OV8865_BLC_CTRL1_COL_SHIFT_128 (1 << 4)
>>> -#define OV8865_BLC_CTRL1_COL_SHIFT_64 (2 << 4)
>>> -#define OV8865_BLC_CTRL1_COL_SHIFT_32 (3 << 4)
>>> #define OV8865_BLC_CTRL1_OFFSET_LIMIT_EN BIT(2)
>>> -#define OV8865_BLC_CTRL1_COLUMN_CANCEL_EN BIT(1)
>>> -#define OV8865_BLC_CTRL2_REG 0x4002
>>> -#define OV8865_BLC_CTRL3_REG 0x4003
>>> -#define OV8865_BLC_CTRL4_REG 0x4004
>>> -#define OV8865_BLC_CTRL5_REG 0x4005
>>> -#define OV8865_BLC_CTRL6_REG 0x4006
>>> -#define OV8865_BLC_CTRL7_REG 0x4007
>>> -#define OV8865_BLC_CTRL8_REG 0x4008
>>> -#define OV8865_BLC_CTRL9_REG 0x4009
>>> -#define OV8865_BLC_CTRLA_REG 0x400a
>>> -#define OV8865_BLC_CTRLB_REG 0x400b
>>> -#define OV8865_BLC_CTRLC_REG 0x400c
>>> #define OV8865_BLC_CTRLD_REG 0x400d
>>> #define OV8865_BLC_CTRLD_OFFSET_TRIGGER(v) ((v) & GENMASK(7, 0))
>>>
>>> @@ -337,66 +268,8 @@
>>>
>>> /* MIPI */
>>>
>>> -#define OV8865_MIPI_CTRL0_REG 0x4800
>>> -#define OV8865_MIPI_CTRL1_REG 0x4801
>>> -#define OV8865_MIPI_CTRL2_REG 0x4802
>>> -#define OV8865_MIPI_CTRL3_REG 0x4803
>>> -#define OV8865_MIPI_CTRL4_REG 0x4804
>>> -#define OV8865_MIPI_CTRL5_REG 0x4805
>>> -#define OV8865_MIPI_CTRL6_REG 0x4806
>>> -#define OV8865_MIPI_CTRL7_REG 0x4807
>>> -#define OV8865_MIPI_CTRL8_REG 0x4808
>>> -
>>> -#define OV8865_MIPI_FCNT_MAX_H_REG 0x4810
>>> -#define OV8865_MIPI_FCNT_MAX_L_REG 0x4811
>>> -
>>> -#define OV8865_MIPI_CTRL13_REG 0x4813
>>> -#define OV8865_MIPI_CTRL14_REG 0x4814
>>> -#define OV8865_MIPI_CTRL15_REG 0x4815
>>> -#define OV8865_MIPI_EMBEDDED_DT_REG 0x4816
>>> -
>>> -#define OV8865_MIPI_HS_ZERO_MIN_H_REG 0x4818
>>> -#define OV8865_MIPI_HS_ZERO_MIN_L_REG 0x4819
>>> -#define OV8865_MIPI_HS_TRAIL_MIN_H_REG 0x481a
>>> -#define OV8865_MIPI_HS_TRAIL_MIN_L_REG 0x481b
>>> -#define OV8865_MIPI_CLK_ZERO_MIN_H_REG 0x481c
>>> -#define OV8865_MIPI_CLK_ZERO_MIN_L_REG 0x481d
>>> -#define OV8865_MIPI_CLK_PREPARE_MAX_REG 0x481e
>>> -#define OV8865_MIPI_CLK_PREPARE_MIN_REG 0x481f
>>> -#define OV8865_MIPI_CLK_POST_MIN_H_REG 0x4820
>>> -#define OV8865_MIPI_CLK_POST_MIN_L_REG 0x4821
>>> -#define OV8865_MIPI_CLK_TRAIL_MIN_H_REG 0x4822
>>> -#define OV8865_MIPI_CLK_TRAIL_MIN_L_REG 0x4823
>>> -#define OV8865_MIPI_LPX_P_MIN_H_REG 0x4824
>>> -#define OV8865_MIPI_LPX_P_MIN_L_REG 0x4825
>>> -#define OV8865_MIPI_HS_PREPARE_MIN_REG 0x4826
>>> -#define OV8865_MIPI_HS_PREPARE_MAX_REG 0x4827
>>> -#define OV8865_MIPI_HS_EXIT_MIN_H_REG 0x4828
>>> -#define OV8865_MIPI_HS_EXIT_MIN_L_REG 0x4829
>>> -#define OV8865_MIPI_UI_HS_ZERO_MIN_REG 0x482a
>>> -#define OV8865_MIPI_UI_HS_TRAIL_MIN_REG 0x482b
>>> -#define OV8865_MIPI_UI_CLK_ZERO_MIN_REG 0x482c
>>> -#define OV8865_MIPI_UI_CLK_PREPARE_REG 0x482d
>>> -#define OV8865_MIPI_UI_CLK_POST_MIN_REG 0x482e
>>> -#define OV8865_MIPI_UI_CLK_TRAIL_MIN_REG 0x482f
>>> -#define OV8865_MIPI_UI_LPX_P_MIN_REG 0x4830
>>> -#define OV8865_MIPI_UI_HS_PREPARE_REG 0x4831
>>> -#define OV8865_MIPI_UI_HS_EXIT_MIN_REG 0x4832
>>> -#define OV8865_MIPI_PKT_START_SIZE_REG 0x4833
>>> -
>>> #define OV8865_MIPI_PCLK_PERIOD_REG 0x4837
>>> -#define OV8865_MIPI_LP_GPIO0_REG 0x4838
>>> -#define OV8865_MIPI_LP_GPIO1_REG 0x4839
>>> -
>>> -#define OV8865_MIPI_CTRL3C_REG 0x483c
>>> -#define OV8865_MIPI_LP_GPIO4_REG 0x483d
>>> -
>>> -#define OV8865_MIPI_CTRL4A_REG 0x484a
>>> -#define OV8865_MIPI_CTRL4B_REG 0x484b
>>> -#define OV8865_MIPI_CTRL4C_REG 0x484c
>>> -#define OV8865_MIPI_LANE_TEST_PATTERN_REG 0x484d
>>> -#define OV8865_MIPI_FRAME_END_DELAY_REG 0x484e
>>> -#define OV8865_MIPI_CLOCK_TEST_PATTERN_REG 0x484f
>>> +
>>> #define OV8865_MIPI_LANE_SEL01_REG 0x4850
>>> #define OV8865_MIPI_LANE_SEL01_LANE0(v) (((v) << 0) &
>>> GENMASK(2, 0))
>>> #define OV8865_MIPI_LANE_SEL01_LANE1(v) (((v) << 4) &
>>> GENMASK(6, 4))
>>> @@ -407,7 +280,6 @@
>>> /* ISP */
>>>
>>> #define OV8865_ISP_CTRL0_REG 0x5000
>>> -#define OV8865_ISP_CTRL0_LENC_EN BIT(7)
>>> #define OV8865_ISP_CTRL0_WHITE_BALANCE_EN BIT(4)
>>> #define OV8865_ISP_CTRL0_DPC_BLACK_EN BIT(2)
>>> #define OV8865_ISP_CTRL0_DPC_WHITE_EN BIT(1)
>>> @@ -416,17 +288,11 @@
>>> #define OV8865_ISP_CTRL2_REG 0x5002
>>> #define OV8865_ISP_CTRL2_DEBUG BIT(3)
>>> #define OV8865_ISP_CTRL2_VARIOPIXEL_EN BIT(2)
>>> -#define OV8865_ISP_CTRL2_VSYNC_LATCH_EN BIT(0)
>>> -#define OV8865_ISP_CTRL3_REG 0x5003
>>>
>>> #define OV8865_ISP_GAIN_RED_H_REG 0x5018
>>> #define OV8865_ISP_GAIN_RED_H(v) (((v) & GENMASK(13, 6)) >>
>>> 6)
>>> #define OV8865_ISP_GAIN_RED_L_REG 0x5019
>>> #define OV8865_ISP_GAIN_RED_L(v) ((v) & GENMASK(5, 0))
>>> -#define OV8865_ISP_GAIN_GREEN_H_REG 0x501a
>>> -#define OV8865_ISP_GAIN_GREEN_H(v) (((v) & GENMASK(13, 6)) >>
>>> 6)
>>> -#define OV8865_ISP_GAIN_GREEN_L_REG 0x501b
>>> -#define OV8865_ISP_GAIN_GREEN_L(v) ((v) & GENMASK(5, 0))
>>> #define OV8865_ISP_GAIN_BLUE_H_REG 0x501c
>>> #define OV8865_ISP_GAIN_BLUE_H(v) (((v) & GENMASK(13, 6)) >>
>>> 6)
>>> #define OV8865_ISP_GAIN_BLUE_L_REG 0x501d
>>> @@ -434,7 +300,6 @@
>>>
>>> /* VarioPixel */
>>>
>>> -#define OV8865_VAP_CTRL0_REG 0x5900
>>> #define OV8865_VAP_CTRL1_REG 0x5901
>>> #define OV8865_VAP_CTRL1_HSUB_COEF(v) ((((v) - 1) << 2) & \
>>> GENMASK(3, 2))
>>> --
>>> 2.25.1
>>>
>>>
>> --
>> With Best Regards,
>> Andy Shevchenko
Hi Paul
On 23/07/2021 08:49, Paul Kocialkowski wrote:
> Hi Daniel,
>
> On Thu 22 Jul 21, 21:33, Daniel Scally wrote:
>> The ov8865 driver is one of those that can be connected to a CIO2
>> device by the cio2-bridge code. This means that the absence of an
>> endpoint for this device is not necessarily fatal, as one might be
>> built by the cio2-bridge when it probes. Return -EPROBE_DEFER if no
>> endpoint is found rather than a fatal error.
> Is this an error that you have actually seen in practice?
Yes
> My understanding is that this function should return the handle to the *local*
> fwnode graph endpoint, which relates to the static device-tree description
> and should be unrelated to another driver probing.
>
> So as far as I can see, this should not be needed (but correct me if I'm wrong).
It's a newly possible scenario - some laptops with Intel IPU3s have ACPI
that is _creative_,. and rather than define the endpoints in an ACPI
device's _DSD they're encoded within a buffer that's against the sensor
devices instead. We're parsing those out and building the fwnode graph
using software nodes in the ipu3-cio2 driver:
https://elixir.bootlin.com/linux/v5.14-rc2/source/drivers/media/pci/intel/ipu3/cio2-bridge.c#L166
But that means there's no endpoint for the sensor until ipu3-cio2 has
probed.
Thanks
Dan
>
> Cheers,
>
> Paul
>
>> Signed-off-by: Daniel Scally <[email protected]>
>> ---
>> drivers/media/i2c/ov8865.c | 6 ++----
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c
>> index 2ef146e7e7ef..66182142c28b 100644
>> --- a/drivers/media/i2c/ov8865.c
>> +++ b/drivers/media/i2c/ov8865.c
>> @@ -2796,10 +2796,8 @@ static int ov8865_probe(struct i2c_client *client)
>> /* Graph Endpoint */
>>
>> handle = fwnode_graph_get_next_endpoint(dev_fwnode(dev), NULL);
>> - if (!handle) {
>> - dev_err(dev, "unable to find endpoint node\n");
>> - return -EINVAL;
>> - }
>> + if (!handle)
>> + return -EPROBE_DEFER;
>>
>> sensor->endpoint.bus_type = V4L2_MBUS_CSI2_DPHY;
>>
>> --
>> 2.25.1
>>
On 23/07/2021 08:44, Paul Kocialkowski wrote:
> Hi,
>
> On Thu 22 Jul 21, 21:33, Daniel Scally wrote:
>> The PLL configuration defined here sets 72MHz (which is correct), not
>> 80MHz. Correct the comment.
> This is:
>
> Reviewed-by: Paul Kocialkowski <[email protected]>
>
> Thanks,
Thank you :)
>
> Paul
>
>> Signed-off-by: Daniel Scally <[email protected]>
>> ---
>> drivers/media/i2c/ov8865.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c
>> index fe60cda3dea7..2ef146e7e7ef 100644
>> --- a/drivers/media/i2c/ov8865.c
>> +++ b/drivers/media/i2c/ov8865.c
>> @@ -713,7 +713,7 @@ static const struct ov8865_pll2_config ov8865_pll2_config_native = {
>> /*
>> * EXTCLK = 24 MHz
>> * DAC_CLK = 360 MHz
>> - * SCLK = 80 MHz
>> + * SCLK = 72 MHz
>> */
>>
>> static const struct ov8865_pll2_config ov8865_pll2_config_binning = {
>> --
>> 2.25.1
>>
Hi Paul
On 23/07/2021 08:57, Paul Kocialkowski wrote:
> Hi,
>
> On Thu 22 Jul 21, 21:34, Daniel Scally wrote:
>> The V4L2_CID_GAIN control for this driver configures registers that
>> the datasheet specifies as analogue gain. Switch the control's ID
>> to V4L2_CID_ANALOGUE_GAIN.
> I had some doubts about this when writing the driver because it's called
> "AEC gain" but it seems that you're right. The datasheet also defines
> 0x350a and 0x350b as digital gain (which are unused by the driver).
>
> This is:
> Reviewed-by: Paul Kocialkowski <[email protected]>
Thanks - yeah it took me a while to be sure too. Particularly confusing
because it's called "AEC gain" in a section headed "Manual exposure /
gain compensation"...they do like to make the datasheets clear as mud
sometimes.
>
> Cheers,
>
> Paul
>
>> Signed-off-by: Daniel Scally <[email protected]>
>> ---
>> drivers/media/i2c/ov8865.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c
>> index c012f5cb11ab..09558a3342dd 100644
>> --- a/drivers/media/i2c/ov8865.c
>> +++ b/drivers/media/i2c/ov8865.c
>> @@ -2137,7 +2137,7 @@ static int ov8865_exposure_configure(struct ov8865_sensor *sensor, u32 exposure)
>>
>> /* Gain */
>>
>> -static int ov8865_gain_configure(struct ov8865_sensor *sensor, u32 gain)
>> +static int ov8865_analog_gain_configure(struct ov8865_sensor *sensor, u32 gain)
>> {
>> int ret;
>>
>> @@ -2447,8 +2447,8 @@ static int ov8865_s_ctrl(struct v4l2_ctrl *ctrl)
>> if (ret)
>> return ret;
>> break;
>> - case V4L2_CID_GAIN:
>> - ret = ov8865_gain_configure(sensor, ctrl->val);
>> + case V4L2_CID_ANALOGUE_GAIN:
>> + ret = ov8865_analog_gain_configure(sensor, ctrl->val);
>> if (ret)
>> return ret;
>> break;
>> @@ -2493,7 +2493,7 @@ static int ov8865_ctrls_init(struct ov8865_sensor *sensor)
>>
>> /* Gain */
>>
>> - v4l2_ctrl_new_std(handler, ops, V4L2_CID_GAIN, 128, 8191, 128, 128);
>> + v4l2_ctrl_new_std(handler, ops, V4L2_CID_ANALOGUE_GAIN, 128, 8191, 128, 128);
>>
>> /* White Balance */
>>
>> --
>> 2.25.1
>>