Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 14 Apr 2022 11:22:39 +0300
From:   Sakari Ailus <sakari.ailus@iki.fi>
To:     Moses Christopher Bollavarapu <mosescb.dev@gmail.com>
Cc:     mchehab@kernel.org, linux-media@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] media: ov7640: Use ARRAY_SIZE instead of manual checking
Message-ID: <YlfZz+j/KSl1joHT@valkosipuli.retiisi.eu>
References: <20220317144714.47442-1-mosescb.dev@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220317144714.47442-1-mosescb.dev@gmail.com>
Precedence: bulk

Hi Moses,

On Thu, Mar 17, 2022 at 03:47:14PM +0100, Moses Christopher Bollavarapu wrote:
> Currently, the driver ends the reg-val list with a 0xFF as a check to stop
> the loop. Instead an array of reg-vals can be used to avoid this check,
> by using the ARRAY_SIZE(arr) macro to obtain the length of the array and
> iterate over it.
> 
> Signed-off-by: Moses Christopher Bollavarapu <mosescb.dev@gmail.com>
> ---
>  drivers/media/i2c/ov7640.c | 33 +++++++++++++++++++--------------
>  1 file changed, 19 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov7640.c b/drivers/media/i2c/ov7640.c
> index 010803d58ce8..977cd2d8ad33 100644
> --- a/drivers/media/i2c/ov7640.c
> +++ b/drivers/media/i2c/ov7640.c
> @@ -13,23 +13,28 @@
>  MODULE_DESCRIPTION("OmniVision ov7640 sensor driver");
>  MODULE_LICENSE("GPL v2");
>  
> -static const u8 initial_registers[] = {
> -	0x12, 0x80,
> -	0x12, 0x54,
> -	0x14, 0x24,
> -	0x15, 0x01,
> -	0x28, 0x20,
> -	0x75, 0x82,
> -	0xFF, 0xFF, /* Terminator (reg 0xFF is unused) */
> +struct reg_val {
> +	u8 reg;
> +	u8 val;
>  };
>  
> -static int write_regs(struct i2c_client *client, const u8 *regs)
> -{
> -	int i;
> +static const struct reg_val regval_init[] = {
> +	{0x12, 0x80},
> +	{0x12, 0x54},
> +	{0x14, 0x24},
> +	{0x15, 0x01},
> +	{0x28, 0x20},
> +	{0x75, 0x82},
> +};
>  
> -	for (i = 0; regs[i] != 0xFF; i += 2)
> -		if (i2c_smbus_write_byte_data(client, regs[i], regs[i + 1]) < 0)
> +static int write_regs(struct i2c_client *client,
> +		const struct reg_val *rv, int len)

Please align to opening parenthesis on the next time. Somehow checkpatch.pl
doesn't seem to complain about this. I.e.

... func(arg1, ...,
	 arg2);

Applied.

> +{
> +	while (--len >= 0) {
> +		if (i2c_smbus_write_byte_data(client, rv->reg, rv->val) < 0)
>  			return -1;
> +		rv++;
> +	}
>  	return 0;
>  }
>  
> @@ -56,7 +61,7 @@ static int ov7640_probe(struct i2c_client *client,
>  	v4l_info(client, "chip found @ 0x%02x (%s)\n",
>  			client->addr << 1, client->adapter->name);
>  
> -	if (write_regs(client, initial_registers) < 0) {
> +	if (write_regs(client, regval_init, ARRAY_SIZE(regval_init)) < 0) {
>  		v4l_err(client, "error initializing OV7640\n");
>  		return -ENODEV;
>  	}

-- 
Regards,

Sakari Ailus