On Fri, Jul 22, 2022 at 12:25 PM ChiaEn Wu <peterwu.pub@xxxxxxxxx> wrote: > > From: ChiaEn Wu <chiaen_wu@xxxxxxxxxxx> > > MediaTek MT6370 is a SubPMIC consisting of a single cell battery charger > with ADC monitoring, RGB LEDs, dual channel flashlight, WLED backlight > driver, display bias voltage supply, one general purpose LDO, and the > USB Type-C & PD controller complies with the latest USB Type-C and PD > standards. > > Add a support the MT6370 ADC driver for system monitoring, including support for the > charger current, voltage, and temperature. ... > +#define MT6370_AICR_400_mA 0x6 > +#define MT6370_ICHG_500_mA 0x4 > +#define MT6370_ICHG_900_mA 0x8 ^^^^ (Note this and read below) ... > + reg_val = FIELD_GET(MT6370_AICR_ICHG_MASK, reg_val); > + if (reg_val < MT6370_AICR_400_mA) > + *val1 = 3350; > + else > + *val1 = 5000; Here... ... > + if (reg_val < MT6370_ICHG_500_mA) > + *val1 = 2375; > + else if (reg_val >= MT6370_ICHG_500_mA && > + reg_val < MT6370_ICHG_900_mA) > + *val1 = 2680; > + else > + *val1 = 5000; ...and especially here it is so counterintuitive to have an if-else chain because the values are not ordered by semantic meaning. What if the new standard/hardware decides to use 0x7 for 100mA (hypothetically)? So, please use switch cases or other robust methods. -- With Best Regards, Andy Shevchenko
