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