On Thu, Apr 14, 2016 at 04:32:04PM +0900, James Bans wrote: A couple of minor points but otherwise this looks good: > +static const int pv88080_buck2_limits[] = { > + 1496000, 2393000, 3291000, 4189000 > +}; > + > +static const int pv88080_buck3_limits[] = { > + 1496000, 2393000, 3291000, 4189000 > +}; These two appear identical so should just be one array. > + switch (data & PV88080_BUCK1_MODE_MASK) { > + case PV88080_BUCK_MODE_SYNC: > + mode = REGULATOR_MODE_FAST; > + break; > + case PV88080_BUCK_MODE_AUTO: > + mode = REGULATOR_MODE_NORMAL; > + break; > + case PV88080_BUCK_MODE_SLEEP: > + mode = REGULATOR_MODE_STANDBY; > + break; > + } I know mode was initialized above but this would be clearer with a default: case.