Subject: [PATCH] media: ov7640: Use ARRAY_SIZE instead of manual checking

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 <[email protected]>
---
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)
+{
+ 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;
}
--
2.30.2


2022-04-15 02:49:47

by Sakari Ailus

[permalink] [raw]
Subject: Re: [PATCH] media: ov7640: Use ARRAY_SIZE instead of manual checking

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 <[email protected]>
> ---
> 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