On Tue, Aug 24, 2021 at 03:42:58PM +0300, Andy Shevchenko wrote:
> 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 :-)
Agree, just tested with:
---
#include <stdio.h>
int main() {
signed int lower_thres, upper_thres, lower, upper;
int val;
for (val = 0; val <= 1000; val+=500) {
lower = -val & 0xFFF;
upper = val & 0x7FF;
printf("val %d, upper 0x%x, lower 0x%x\n", val, upper, lower);
}
}
---
val 0, upper 0x0, lower 0x0
val 500, upper 0x1f4, lower 0xe0c
val 1000, upper 0x3e8, lower 0xc18
Thanks :)
/Sean
