Re: [PATCH 1/2] Port rtc-pcf2123 to regmap

[Alexandre Belloni <alexandre.belloni@bootlin.com>]

Sorry for the very late reply !

On 03/05/2019 13:30:12+0000, Dylan Howey wrote:
> The 05/02/2019 22:55, Alexandre Belloni wrote:
> > > * I don't think pcf2123_read_offset is working correctly. In the case of
> > >   a coarse offset the value is not being sign extended. So a negative
> > >   offset read will not be correct if the coarse bit is set (result would
> > >   be a positive number being returned if this is true). I need to look
> > >   into this some more. The fix would be to sign extend first, then if
> > >   coarse bit is set multiply the result by 2.
> > > 
> > 
> > As the comment says, it is properly extended because after shifting,
> > bits [6:0] become bit [7:1].
> > 
> But what about this:
> 
> # echo 2170 > /sys/class/rtc/rtc0/offset 
> # cat /sys/class/rtc/rtc0/offset 
> 2170
> # echo -2170 > /sys/class/rtc/rtc0/offset 
> # cat /sys/class/rtc/rtc0/offset 
> 0
> # echo 4340 > /sys/class/rtc/rtc0/offset 
> # cat /sys/class/rtc/rtc0/offset 
> 4340
> # echo -4340 > /sys/class/rtc/rtc0/offset 
> # cat /sys/class/rtc/rtc0/offset 
> -2170
> 
> Negative offset reads seem to be off by a factor of 2.
> 

The issue is not the sign extension, it is the rounding function that
only works properly for positive numbers:

reg = (s8)((offset + (OFFSET_STEP >> 1)) / OFFSET_STEP);

I'm sending a patch.

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com