Re: [PATCH v3] media: ov8856: Add support for OV8856 sensor

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Ben,

Thanks for the update! I've applied the patch, but please see the comments
below.

On Tue, Jan 15, 2019 at 04:30:29PM +0800, Ben Kao wrote:
...
> +static int ov8856_read_reg(struct ov8856 *ov8856, u16 reg, u16 len, u32 *val)
> +{
> +	struct i2c_client *client = v4l2_get_subdevdata(&ov8856->sd);
> +	struct i2c_msg msgs[2];
> +	u8 addr_buf[2] = {reg >> 8, reg & 0xff};

You could use put_unaligned_be16() for doing this, as you do below.

> +	u8 data_buf[4] = {0, };
> +	int ret;
> +
> +	if (len > 4)
> +		return -EINVAL;
> +
> +	msgs[0].addr = client->addr;
> +	msgs[0].flags = 0;
> +	msgs[0].len = ARRAY_SIZE(addr_buf);

sizeof() would be better suited for the purpose: the length is in bytes.

Could you send a follow-up patch on top of this one?

> +	msgs[0].buf = addr_buf;
> +	msgs[1].addr = client->addr;
> +	msgs[1].flags = I2C_M_RD;
> +	msgs[1].len = len;
> +	msgs[1].buf = &data_buf[4 - len];
> +
> +	ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> +	if (ret != ARRAY_SIZE(msgs))
> +		return -EIO;
> +
> +	*val = get_unaligned_be32(data_buf);
> +
> +	return 0;
> +}

-- 
Regards,

Sakari Ailus
sakari.ailus@xxxxxxxxxxxxxxx



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux