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