One trivial request for a comment. Otherwise (subject to jetlag - which snuck up on me during this review) this looks fine to me.. Will be a few weeks though until the precursor series does the round trip via upstream to get into the branch I'd want to apply this on. Jonathan > > +static int bu27010_get_gain_sel(struct bu27008_data *data, int *sel) > +{ > + int ret, tmp; > + > + /* > + * We always "lock" the gain selectors for all channels to prevent > + * unsupported configs. It does not matter which channel is used > + * we can just return selector from any of them. > + */ > + ret = regmap_read(data->regmap, BU27008_REG_MODE_CONTROL2, sel); > + if (ret) > + return ret; > + > + *sel = FIELD_GET(BU27010_MASK_DATA0_GAIN, *sel); > + > + ret = regmap_read(data->regmap, BU27008_REG_MODE_CONTROL1, &tmp); > + if (ret) > + return ret; > + > + *sel |= FIELD_GET(BU27010_MASK_RGBC_GAIN, tmp) << fls(BU27010_MASK_DATA0_GAIN); Odd enough to perhaps warrant a comment on the maths. > + > + return ret; > +} > +