On Mon, 10 Jun 2024 10:26:29 -0300 João Paulo Gonçalves <jpaulo.silvagoncalves@xxxxxxxxx> wrote: > Hi Jonathan, > > Thanks for the review! > > > > + > > > +static int ads1119_validate_gain(struct ads1119_state *st, int scale, int uscale) > > > +{ > > > + int gain = 1000000 / ((scale * 1000000) + uscale); > > > + > > > + switch (gain) { > > > + case 1: > > > + case 4: > > > + return gain; > > Odd to calculate it if we don't need it > > return MICRO / (scale * MICRO + uscale); > > use constants as it's easy to drop a 0 in these without anyone noticing. > > > > > + default: > > > + return -EINVAL; > > > + } > > > +} > > > + > > Just a minor. I do use the calculated value on write_raw() by storing it as the > new channel gain and would still need to validate it as scale/uscale comes from > userspace. Maybe I can just remove the validate_gain function and do the check > directly on write_raw(). What do you think? If that's the only caller, sure, move it to write raw. Jonathan > > Regards, > João Paulo Gonçalves >