On Tue, Aug 24, 2021 at 3:32 PM Sean Nyekjaer <sean@xxxxxxxxxx> wrote: > On Tue, Aug 24, 2021 at 03:15:28PM +0300, Andy Shevchenko wrote: > > On Tue, Aug 24, 2021 at 2:38 PM Sean Nyekjaer <sean@xxxxxxxxxx> wrote: ... > > > + /* > > > + * Add the same value to the lower-threshold register with a reversed sign > > > + * in 2-complement 12 bit format. > > > + */ > > > + data->lower_thres = (~val & GENMASK(11, 0)) + 1; > > > > This looks suspicious. > > > > 0 => 0xfff + 1 => 0x1000. Is it what is wanted? > > I thought that -val & mask is what you need. > > > > Can you explain more in the comment (maybe with examples) on what is > > coming and what is expected? > > It's a bit messy I know :) > > Some examples: > val = 0 => upper = 0x0, lower = 0x0 > val = 500 => upper = 0x1F4, lower = 0xe0c > val = 1000 => upper = 0x3e8, lower = 0xc18 > > Guess it could work if we special case val = 0... > > It doesn't even makes sense to write 0 to this register as noise would > trigger events. > > > > + data->upper_thres = val & GENMASK(10, 0); So, I just tested all three and with '-' (minus) it works, while your code is buggy :-) -- With Best Regards, Andy Shevchenko
