[PATCH 00/13] Extensions to ov8865 driver

[PATCH 00/13] Extensions to ov8865 driver

[Daniel Scally]

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

[PATCH 01/13] media: i2c: Add ACPI support to ov8865

[Daniel Scally]

The ov8865 sensor is sometimes found on x86 platforms enumerated via ACPI.
Add an ACPI match table to the driver so that it's probed on those
platforms.

Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
 drivers/media/i2c/ov8865.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c
index ce50f3ea87b8..fe60cda3dea7 100644
--- a/drivers/media/i2c/ov8865.c
+++ b/drivers/media/i2c/ov8865.c
@@ -5,6 +5,7 @@
  * Author: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
  */
 
+#include <linux/acpi.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/device.h>
@@ -2946,6 +2947,12 @@ static const struct dev_pm_ops ov8865_pm_ops = {
 	SET_RUNTIME_PM_OPS(ov8865_suspend, ov8865_resume, NULL)
 };
 
+static const struct acpi_device_id ov8865_acpi_match[] = {
+	{"INT347A"},
+	{},
+};
+MODULE_DEVICE_TABLE(acpi, ov8865_acpi_match);
+
 static const struct of_device_id ov8865_of_match[] = {
 	{ .compatible = "ovti,ov8865" },
 	{ }
@@ -2956,6 +2963,7 @@ static struct i2c_driver ov8865_driver = {
 	.driver = {
 		.name = "ov8865",
 		.of_match_table = ov8865_of_match,
+		.acpi_match_table = ov8865_acpi_match,
 		.pm = &ov8865_pm_ops,
 	},
 	.probe_new = ov8865_probe,
-- 
2.25.1

Re: [PATCH 01/13] media: i2c: Add ACPI support to ov8865

[Daniel Scally]

On 22/07/2021 22:58, Andy Shevchenko wrote:
>
>
> On Thursday, July 22, 2021, Daniel Scally <djrscally@gmail.com
> <mailto:djrscally@gmail.com>> wrote:
>
>     The ov8865 sensor is sometimes found on x86 platforms enumerated
>     via ACPI.
>     Add an ACPI match table to the driver so that it's probed on those
>     platforms.
>
>     Signed-off-by: Daniel Scally <djrscally@gmail.com
>     <mailto:djrscally@gmail.com>>
>     ---
>      drivers/media/i2c/ov8865.c | 8 ++++++++
>      1 file changed, 8 insertions(+)
>
>     diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c
>     index ce50f3ea87b8..fe60cda3dea7 100644
>     --- a/drivers/media/i2c/ov8865.c
>     +++ b/drivers/media/i2c/ov8865.c
>     @@ -5,6 +5,7 @@
>       * Author: Paul Kocialkowski <paul.kocialkowski@bootlin.com
>     <mailto:paul.kocialkowski@bootlin.com>>
>       */
>
>     +#include <linux/acpi.h>
>
>
>
> No user of it. Probably you meant mod_devicetable.h


Huh, I guess I did yeah...for some reason I thought acpi_device_id was
in acpi.h. Thanks very much; I'll fix it.

>  
>
>      #include <linux/clk.h>
>      #include <linux/delay.h>
>      #include <linux/device.h>
>     @@ -2946,6 +2947,12 @@ static const struct dev_pm_ops
>     ov8865_pm_ops = {
>             SET_RUNTIME_PM_OPS(ov8865_suspend, ov8865_resume, NULL)
>      };
>
>     +static const struct acpi_device_id ov8865_acpi_match[] = {
>     +       {"INT347A"},
>     +       {},
>     +};
>     +MODULE_DEVICE_TABLE(acpi, ov8865_acpi_match);
>     +
>      static const struct of_device_id ov8865_of_match[] = {
>             { .compatible = "ovti,ov8865" },
>             { }
>     @@ -2956,6 +2963,7 @@ static struct i2c_driver ov8865_driver = {
>             .driver = {
>                     .name = "ov8865",
>                     .of_match_table = ov8865_of_match,
>     +               .acpi_match_table = ov8865_acpi_match,
>                     .pm = &ov8865_pm_ops,
>             },
>             .probe_new = ov8865_probe,
>     -- 
>     2.25.1
>
>
>
> -- 
> With Best Regards,
> Andy Shevchenko
>
>

[PATCH 02/13] media: i2c: Fix incorrect value in comment

[Daniel Scally]

The PLL configuration defined here sets 72MHz (which is correct), not
80MHz. Correct the comment.

Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
 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

Re: [PATCH 02/13] media: i2c: Fix incorrect value in comment

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 990 bytes --]

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 <paul.kocialkowski@bootlin.com>

Thanks,

Paul
 
> Signed-off-by: Daniel Scally <djrscally@gmail.com>
> ---
>  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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 02/13] media: i2c: Fix incorrect value in comment

[Daniel Scally]

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 <paul.kocialkowski@bootlin.com>
>
> Thanks,


Thank you :)

>
> Paul
>  
>> Signed-off-by: Daniel Scally <djrscally@gmail.com>
>> ---
>>  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
>>

[PATCH 03/13] media: i2c: Defer probe if not endpoint found

[Daniel Scally]

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 <djrscally@gmail.com>
---
 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

Re: [PATCH 03/13] media: i2c: Defer probe if not endpoint found

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 1616 bytes --]

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 <djrscally@gmail.com>
> ---
>  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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 03/13] media: i2c: Defer probe if not endpoint found

[Daniel Scally]

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 <djrscally@gmail.com>
>> ---
>>  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
>>

[PATCH 04/13] media: i2c: Support 19.2MHz input clock in ov8865

[Daniel Scally]

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 <djrscally@gmail.com>
---
 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

[PATCH 05/13] media: i2c: Add .get_selection() support to ov8865

[Daniel Scally]

The ov8865 driver's v4l2_subdev_pad_ops currently does not include
.get_selection() - add support for that callback.

Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
 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

Re: [PATCH 05/13] media: i2c: Add .get_selection() support to ov8865

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2072 bytes --]

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 <lkp@intel.com>

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/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 41774 bytes --]

[PATCH 06/13] media: i2c: Switch control to V4L2_CID_ANALOGUE_GAIN

[Daniel Scally]

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 <djrscally@gmail.com>
---
 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

Re: [PATCH 06/13] media: i2c: Switch control to V4L2_CID_ANALOGUE_GAIN

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 1962 bytes --]

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 <paul.kocialkowski@bootlin.com>

Cheers,

Paul

> Signed-off-by: Daniel Scally <djrscally@gmail.com>
> ---
>  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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 06/13] media: i2c: Switch control to V4L2_CID_ANALOGUE_GAIN

[Daniel Scally]

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 <paul.kocialkowski@bootlin.com>


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 <djrscally@gmail.com>
>> ---
>>  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
>>

[PATCH 07/13] media: i2c: Add vblank control to ov8865

[Daniel Scally]

Add a V4L2_CID_VBLANK control to the ov8865 driver.

Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
 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

[PATCH 08/13] media: i2c: Add hblank control to ov8865

[Daniel Scally]

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 <djrscally@gmail.com>
---

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

[PATCH 09/13] media: i2c: cap exposure at height + vblank in ov8865

[Daniel Scally]

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 <djrscally@gmail.com>
---
 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

[PATCH 10/13] media: i2c: Add controls from fwnode to ov8865

[Daniel Scally]

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 <djrscally@gmail.com>
---
 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

[PATCH 11/13] media: i2c: Switch exposure control unit to lines

[Daniel Scally]

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 <djrscally@gmail.com>
---
 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

[PATCH 12/13] media: i2c: Remove unused macros from ov8865

[Daniel Scally]

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 <djrscally@gmail.com>
---

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

Re: [PATCH 12/13] media: i2c: Remove unused macros from ov8865

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 15497 bytes --]

Hi,

On Fri 23 Jul 21, 01:19, Andy Shevchenko wrote:
> On Thursday, July 22, 2021, Daniel Scally <djrscally@gmail.com> 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 <djrscally@gmail.com>
> > ---
> >
> > 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 12/13] media: i2c: Remove unused macros from ov8865

[Daniel Scally]

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 <djrscally@gmail.com> 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 <djrscally@gmail.com>
>>> ---
>>>
>>> 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

[PATCH 13/13] media: ipu3-cio2: Add INT347A to cio2-bridge

[Daniel Scally]

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 <djrscally@gmail.com>
---
 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

Re: [PATCH 13/13] media: ipu3-cio2: Add INT347A to cio2-bridge

[Daniel Scally]

Hi Andy

On 22/07/2021 23:22, Andy Shevchenko wrote:
>
>
> On Thursday, July 22, 2021, Daniel Scally <djrscally@gmail.com
> <mailto:djrscally@gmail.com>> 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 <djrscally@gmail.com
>     <mailto:djrscally@gmail.com>>
>     ---
>      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
>
>