2018年7月10日(火) 6:23 Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxxxx>: > > Hi, > > On Mon, Jul 09, 2018 at 06:14:43PM +0200, Wolfram Sang wrote: > > > static int ov772x_read(struct i2c_client *client, u8 addr) > > > { > > > - int ret; > > > - u8 val; > > > - > > > - ret = i2c_master_send(client, &addr, 1); > > > - if (ret < 0) > > > - return ret; > > > - ret = i2c_master_recv(client, &val, 1); > > > - if (ret < 0) > > > - return ret; > > > - > > > - return val; > > > + return sccb_read_byte(client, addr); > > > } > > > > > > static inline int ov772x_write(struct i2c_client *client, u8 addr, u8 value) > > > { > > > - return i2c_smbus_write_byte_data(client, addr, value); > > > + return sccb_write_byte(client, addr, value); > > > } > > Reviewed-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxxxx> > > > Minor nit: I'd rather drop these two functions and use the > > sccb-accessors directly. > > > > However, I really like how this looks here: It is totally clear we are > > doing SCCB and hide away all the details. > > I think it would be even better to introduce a SSCB regmap layer > and use that. I'm fine with introducing a SCCB regmap layer. But do we need to provide both a SCCB regmap and these SCCB helpers? If we have a regmap_init_sccb(), I feel these three SCCB helper functions don't need to be exported anymore.