On Sat, Jun 18, 2022 at 11:40 PM Aidan MacDonald <aidanmacdonald.0x0@xxxxxxxxx> wrote: > > The AXP192 is mostly the same as the AXP202 but has a different > current limit. ... > + int ret = regmap_read(power->regmap, AXP20X_VBUS_IPSOUT_MGMT, &v); > + > + if (ret) > + return ret; Please, split assignment, so it will be like int ret; ret = ... if (ret) ... > +static int axp192_usb_power_set_current_max(struct axp20x_usb_power *power, > + int intval) > +{ > + int val = AXP192_VBUS_CLIMIT_EN; > + const int mask = AXP192_VBUS_CLIMIT_EN | AXP192_VBUS_CLIMIT_100mA; > + > + switch (intval) { > + case 100000: > + val |= AXP192_VBUS_CLIMIT_100mA; > + fallthrough; It's harder to part and error prone, can't you simply call a regmap_update_bits with different arguments? With this suggestion consider the defaults like these: const int mask = AXP192_VBUS_CLIMIT_EN | AXP192_VBUS_CLIMIT_100mA; int val = mask; Then use val & ~AXP192_VBUS_CLIMIT_100mA in the below case. > + case 500000: > + return regmap_update_bits(power->regmap, > + AXP20X_VBUS_IPSOUT_MGMT, mask, val); > + default: > + return -EINVAL; > + } > +} -- With Best Regards, Andy Shevchenko