Re: [PATCH v3 3/5] media: ov02c10: Split the PLL registers off the mode tables

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Felipe,

On Wed, Sep 23, 2026 at 08:40:58PM -0300, Felipe Calliari wrote:
> The PLL is programmed partly by the common mode table and partly by the
> per-lane tables, which override 0x0303 and 0x0305 with the same values
> for either lane count and also set 0x0316.
> 
> Move the 0x03xx PLL registers into a table of their own, keeping only
> the values that end up in the sensor, and write it before the mode
> tables. The PLL settings depend on the external clock rate rather than
> on the mode or the lane count, and this prepares for supporting clock
> rates other than 19.2 MHz.
> 
> No functional change intended.
> 
> Signed-off-by: Felipe Calliari <calliarifelipe@gmail.com>
> ---
> 
> Notes:
>     Changes in v3:
>     - New patch, as asked by Sakari.
> 
>  drivers/media/i2c/ov02c10.c | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
> index cdccbdef3..0e5010303 100644
> --- a/drivers/media/i2c/ov02c10.c
> +++ b/drivers/media/i2c/ov02c10.c
> @@ -89,12 +89,6 @@ struct ov02c10_mode {
>  };
>  
>  static const struct reg_sequence sensor_1928x1092_30fps_setting[] = {
> -	{0x0301, 0x08},
> -	{0x0303, 0x06},

I haven't tried to figure out what this register exatly is, but this patch
changes how this register is programmed. After this patch, the value of
this register is always 5.

> -	{0x0304, 0x01},
> -	{0x0305, 0xe0},
> -	{0x0313, 0x40},
> -	{0x031c, 0x4f},
>  	{0x3020, 0x97},
>  	{0x3022, 0x01},
>  	{0x3026, 0xb4},
> @@ -303,10 +297,6 @@ static const struct reg_sequence sensor_1928x1092_30fps_1lane_setting[] = {
>  	{0x394e, 0x0b},
>  	{0x4800, 0x24},
>  	{0x5000, 0xf5},
> -	/* plls */
> -	{0x0303, 0x05},
> -	{0x0305, 0x90},
> -	{0x0316, 0x90},
>  	{0x3016, 0x12},
>  };
>  
> @@ -330,11 +320,17 @@ static const struct reg_sequence sensor_1928x1092_30fps_2lane_setting[] = {
>  	{0x4d0d, 0x00},
>  	{0x5000, 0xfd},
>  	{0x481f, 0x30},
> -	/* plls */
> +	{0x3016, 0x32},
> +};
> +
> +static const struct reg_sequence sensor_pll_19_2mhz_setting[] = {

I guess some of these settings are determining link frequency or pixel
rate, so they're not generic to anything using 19,2 MHz external clock.

> +	{0x0301, 0x08},
>  	{0x0303, 0x05},
> +	{0x0304, 0x01},
>  	{0x0305, 0x90},
> +	{0x0313, 0x40},
>  	{0x0316, 0x90},
> -	{0x3016, 0x32},
> +	{0x031c, 0x4f},
>  };
>  
>  static const char * const ov02c10_test_pattern_menu[] = {
> @@ -601,6 +597,14 @@ static int ov02c10_enable_streams(struct v4l2_subdev *sd,
>  	if (ret)
>  		return ret;
>  
> +	ret = regmap_multi_reg_write(ov02c10->regmap,
> +				     sensor_pll_19_2mhz_setting,
> +				     ARRAY_SIZE(sensor_pll_19_2mhz_setting));
> +	if (ret) {
> +		dev_err(ov02c10->dev, "failed to write PLL settings\n");
> +		goto out;
> +	}
> +
>  	reg_sequence = mode->reg_sequence;
>  	sequence_length = mode->sequence_length;
>  	ret = regmap_multi_reg_write(ov02c10->regmap,

-- 
Kind regards,

Sakari Ailus