Re: [PATCH v4 2/2] media: i2c: Add imx334 camera sensor driver

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

 



Hi Martina,

On Fri, Dec 11, 2020 at 10:56:33AM +0000, Martina Krasteva wrote:
...
> +static int imx334_read_reg(struct imx334 *imx334, u16 reg, u32 len, u32 *val)
> +{
> +	struct i2c_client *client = v4l2_get_subdevdata(&imx334->sd);
> +	struct i2c_msg msgs[2] = { 0 };
> +	u8 addr_buf[2] = { 0 };
> +	u8 data_buf[4] = { 0 };
> +	int ret;
> +
> +	if (WARN_ON(len > 4))
> +		return -EINVAL;
> +
> +	put_unaligned_be16(reg, addr_buf);
> +
> +	/* Write register address */
> +	msgs[0].addr = client->addr;
> +	msgs[0].flags = 0;
> +	msgs[0].len = ARRAY_SIZE(addr_buf);
> +	msgs[0].buf = addr_buf;
> +
> +	/* Read data from register */
> +	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_le32(data_buf + (4 - len));

Hmm. The device native endianness is big (on control interface) unless
something very unexpected happened in hardware development.

You also can't do this as this will overrun data_buf.

> +
> +	return 0;
> +}
> +
> +/**
> + * imx334_write_reg() - Write register
> + * @imx334: pointer to imx334 device
> + * @reg: register address
> + * @len: length of bytes. Max supported bytes is 4
> + * @val: register value
> + *
> + * Return: 0 if successful, error code otherwise.
> + */
> +static int imx334_write_reg(struct imx334 *imx334, u16 reg, u32 len, u32 val)
> +{
> +	struct i2c_client *client = v4l2_get_subdevdata(&imx334->sd);
> +	u8 buf[6] = {0};
> +
> +	if (WARN_ON(len > 4))
> +		return -EINVAL;
> +
> +	put_unaligned_be16(reg, buf);
> +	put_unaligned_le32(val, buf + 2);

Similar comment on this one.

-- 
Kind regards,

Sakari Ailus



[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