Hi Chris, ... > +enum rtl9300_bus_freq { > + RTL9300_I2C_STD_FREQ, > + RTL9300_I2C_FAST_FREQ, > +}; > + > +struct rtl9300_i2c; > + > +struct rtl9300_i2c_chan { > + struct i2c_adapter adap; > + struct rtl9300_i2c *i2c; > + enum rtl9300_bus_freq bus_freq; > + u8 sda_pin; > +}; > + > +#define RTL9300_I2C_MUX_NCHAN 8 > + > +struct rtl9300_i2c { > + struct regmap *regmap; > + struct device *dev; > + struct rtl9300_i2c_chan chans[RTL9300_I2C_MUX_NCHAN]; > + u32 reg_base; > + u8 sda_pin; > +}; ... > +DEFINE_MUTEX(i2c_lock); why is this lock defined here and not inside rtl9300_i2c? ... > +static int rtl9300_i2c_execute_xfer(struct rtl9300_i2c *i2c, char read_write, > + int size, union i2c_smbus_data *data, int len) > +{ > + u32 val, mask; > + int ret; ... > +static int rtl9300_i2c_smbus_xfer(struct i2c_adapter *adap, u16 addr, > + unsigned short flags, char read_write, > + u8 command, int size, union i2c_smbus_data *data) your alignments are a bit off. A part from these two little notes, I don't see any issue here. Thanks, Andi > +{ > + struct rtl9300_i2c_chan *chan = i2c_get_adapdata(adap); > + struct rtl9300_i2c *i2c = chan->i2c; > + int len = 0, ret; ...