On Sun, 23 Sep 2012, Frank Schäfer wrote:
> Signed-off-by: Frank Schäfer <fschaefer.oss@xxxxxxxxxxxxxx>
> ---
> drivers/media/i2c/soc_camera/ov2640.c | 17 ++++++++---------
> 1 Datei geändert, 8 Zeilen hinzugefügt(+), 9 Zeilen entfernt(-)
>
> diff --git a/drivers/media/i2c/soc_camera/ov2640.c b/drivers/media/i2c/soc_camera/ov2640.c
> index 182d5a1..e71bf4c 100644
> --- a/drivers/media/i2c/soc_camera/ov2640.c
> +++ b/drivers/media/i2c/soc_camera/ov2640.c
> @@ -639,17 +639,19 @@ static struct ov2640_priv *to_ov2640(const struct i2c_client *client)
> subdev);
> }
>
> +static int ov2640_write_single(struct i2c_client *client, u8 reg, u8 val)
> +{
> + dev_vdbg(&client->dev, "write: 0x%02x, 0x%02x", reg, val);
> + return i2c_smbus_write_byte_data(client, reg, val);
> +}
Well, I'm not convinced. I don't necessarily see it as a simplification.
You replace one perfectly ok function with another one with exactly the
same parameters. Ok, you also hide a debug printk() in your wrapper, but
that's not too useful either, IMHO. Besides, you're missing more calls to
i2c_smbus_write_byte_data() in ov2640_mask_set(), ov2640_s_register() and
ov2640_video_probe(). So, I'd just drop it.
Thanks
Guennadi
> +
> static int ov2640_write_array(struct i2c_client *client,
> const struct regval_list *vals)
> {
> int ret;
>
> while ((vals->reg_num != 0xff) || (vals->value != 0xff)) {
> - ret = i2c_smbus_write_byte_data(client,
> - vals->reg_num, vals->value);
> - dev_vdbg(&client->dev, "array: 0x%02x, 0x%02x",
> - vals->reg_num, vals->value);
> -
> + ret = ov2640_write_single(client, vals->reg_num, vals->value);
> if (ret < 0)
> return ret;
> vals++;
> @@ -704,13 +706,10 @@ static int ov2640_s_ctrl(struct v4l2_ctrl *ctrl)
> struct v4l2_subdev *sd =
> &container_of(ctrl->handler, struct ov2640_priv, hdl)->subdev;
> struct i2c_client *client = v4l2_get_subdevdata(sd);
> - struct regval_list regval;
> int ret;
> u8 val;
>
> - regval.reg_num = BANK_SEL;
> - regval.value = BANK_SEL_SENS;
> - ret = ov2640_write_array(client, ®val);
> + ret = ov2640_write_single(client, BANK_SEL, BANK_SEL_SENS);
> if (ret < 0)
> return ret;
>
> --
> 1.7.10.4
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
