Re: [PATCH 08/11] media: i2c: ov5640: Document AWB control registers

[Jacopo Mondi <jacopo.mondi@ideasonboard.com>]

On Fri, May 01, 2026 at 04:39:10PM +0100, Kieran Bingham wrote:
> Identify and map the registers that are controlling the AWB and
> document their current impact inline in the register set.
>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  drivers/media/i2c/ov5640.c | 61 +++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 47 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 4b6804fc47e1..34fe7f51e17b 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -112,6 +112,34 @@
>  #define OV5640_REG_PCLK_PERIOD		0x4837
>  #define OV5640_REG_ISP_FORMAT_MUX_CTRL	0x501f
>  #define OV5640_REG_PRE_ISP_TEST_SET1	0x503d
> +
> +#define OV5640_REG_AWB_CONTROL_00	0x5180 /* AWB B block */
> +#define OV5640_REG_AWB_CONTROL_01	0x5181 /* AWB Step and Slope control */
> +#define OV5640_REG_AWB_CONTROL_02	0x5182 /* 7:4 Max local counter 3:0 mas fast counter */

s/mas/max ?

> +#define OV5640_REG_AWB_CONTROL_03	0x5183 /* AWB Simple/Advanced control */
> +#define OV5640_REG_AWB_CONTROL_04	0x5184 /* Count and G enable */
> +#define OV5640_REG_AWB_CONTROL_05	0x5185 /* Stable Range Thresholds */
> +
> +#define OV5640_REG_AWB_CONTROL_17	0x5191 /* AWB Top limit */
> +#define OV5640_REG_AWB_CONTROL_18	0x5192 /* AWB Bottom limit */
> +#define OV5640_REG_AWB_CONTROL_19	0x5193 /* Red limit */
> +#define OV5640_REG_AWB_CONTROL_20	0x5194 /* Green limit */
> +#define OV5640_REG_AWB_CONTROL_21	0x5195 /* Blue limit */
> +
> +#define OV5640_REG_AWB_CONTROL_22	0x5196 /* AWB Freeze and Simple Selection */
> +#define OV5640_AWB_FREEZE		BIT(5) /* AWB freeze */
> +#define OV5640_AWB_SIMPLE_SELECT_MASK	GENMASK(3, 2)
> +#define OV5640_AWB_SIMPLE_AFTER_AWB_0	0 /* AWB simple from after AWB gain */
> +#define OV5640_AWB_SIMPLE_AFTER_GMA_0	1 /* AWB simple from after RAW GMA */
> +#define OV5640_AWB_SIMPLE_AFTER_GMA_1	2 /* AWB simple from after RAW GMA */

That's weird, but that's what the datasheet describes, yes

> +#define OV5640_AWB_SIMPLE_AFTER_AWB_1	3 /* AWB simple from after AWB gain */
> +#define OV5640_AWB_FAST_ENABLE		BIT(1) /* AWB fast enable */
> +#define OV5640_AWB_BIAS_STAT		BIT(0)
> +
> +#define OV5640_REG_AWB_CONTROL_23	0x5197 /* Local Limit */
> +
> +#define OV5640_REG_AWB_CONTROL_30	0x519e /* Local limit and Stable Select */
> +
>  #define OV5640_REG_SDE_CTRL0		0x5580
>  #define OV5640_REG_SDE_CTRL1		0x5581
>  #define OV5640_REG_SDE_CTRL3		0x5583
> @@ -576,12 +604,14 @@ static const struct reg_value ov5640_init_setting[] = {
>  	{0x5000, 0xa7, 0, 0}, {0x5001, 0xa3, 0, 0},
>
>  	/* AWB Control */
> -	{0x5180, 0xff, 0, 0},
> -	{0x5181, 0xf2, 0, 0},
> -	{0x5182, 0x00, 0, 0},
> -	{0x5183, 0x14, 0, 0},
> -	{0x5184, 0x25, 0, 0},
> -	{0x5185, 0x24, 0, 0},
> +	{OV5640_REG_AWB_CONTROL_00, 0xff, 0, 0}, /* AWB B Block */
> +	{OV5640_REG_AWB_CONTROL_01, 0xf2, 0, 0}, /* Step and Slope  - one zone, 0 slope, step fast=step local = 3 */
> +	{OV5640_REG_AWB_CONTROL_02, 0x00, 0, 0}, /* Local/Fast counters @ 0 */
> +	{OV5640_REG_AWB_CONTROL_03, 0x14, 0, 0}, /* Advanced AWB: AWB SIMF, AWB Win = 1 */
> +	{OV5640_REG_AWB_CONTROL_04, 0x25, 0, 0}, /* G-Enable, Count-limit=1, count threshold=1 */
> +	{OV5640_REG_AWB_CONTROL_05, 0x24, 0, 0}, /* Stable Ranges: Threshold for [7:4] unstable to stable [3:0] stable to unstable */
> +
> +	/* AWB Advanced Control - Undocumented */
>  	{0x5186, 0x09, 0, 0},
>  	{0x5187, 0x09, 0, 0},
>  	{0x5188, 0x09, 0, 0},
> @@ -593,20 +623,23 @@ static const struct reg_value ov5640_init_setting[] = {
>  	{0x518e, 0x34, 0, 0},
>  	{0x518f, 0x6b, 0, 0},
>  	{0x5190, 0x46, 0, 0},
> -	{0x5191, 0xf8, 0, 0},
> -	{0x5192, 0x04, 0, 0},
> -	{0x5193, 0x70, 0, 0},
> -	{0x5194, 0xf0, 0, 0},
> -	{0x5195, 0xf0, 0, 0},
> -	{0x5196, 0x03, 0, 0},
> -	{0x5197, 0x01, 0, 0},
> +
> +	{OV5640_REG_AWB_CONTROL_17, 0xf8, 0, 0}, /* AWB Top limit (Default 0xff)*/
> +	{OV5640_REG_AWB_CONTROL_18, 0x04, 0, 0}, /* AWB Bottom limit (Default 0x00) */
> +	{OV5640_REG_AWB_CONTROL_19, 0x70, 0, 0}, /* Red limit (Default 0xf0) */
> +	{OV5640_REG_AWB_CONTROL_20, 0xf0, 0, 0}, /* Green Limit (Default 0xf0) */
> +	{OV5640_REG_AWB_CONTROL_21, 0xf0, 0, 0}, /* Blue limit (Default 0xf0) */
> +	{OV5640_REG_AWB_CONTROL_22, 0x03, 0, 0}, /* AWB after AWB gain; Fast enable; Bias stat; */
> +	{OV5640_REG_AWB_CONTROL_23, 0x01, 0, 0}, /* Local limit (Default 0x02) */
> +
> +	/* Debug mode - Undocumented */
>  	{0x5198, 0x04, 0, 0},
>  	{0x5199, 0x6c, 0, 0},
>  	{0x519a, 0x04, 0, 0},
>  	{0x519b, 0x00, 0, 0},
>  	{0x519c, 0x09, 0, 0},
>  	{0x519d, 0x2b, 0, 0},
> -	{0x519e, 0x38, 0, 0},
> +	{OV5640_REG_AWB_CONTROL_30, 0x38, 0, 0}, /* [7:4] Debug = 3; [3] Local Limit Select = 1; [2] Simple stable select=0; [1:0] Debug=0 */

I'm happy to go over line limit for better documentation.

I anticipate someone might ask to place the comment in the line above
to reduce the line length, but I'm not going to be that person :)

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

>
>  	{0x5381, 0x1e, 0, 0}, {0x5382, 0x5b, 0, 0}, {0x5383, 0x08, 0, 0},
>  	{0x5384, 0x0a, 0, 0}, {0x5385, 0x7e, 0, 0}, {0x5386, 0x88, 0, 0},
>
> --
> 2.52.0
>
>