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

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

On 02/05/2019 17:45:24+0000, Dylan Howey wrote:
> As I'm working on this I've run across some other issues:
> 
> * Driver does not do a software reset on init. Datasheet recommends doing
>   this as this will clear any interrupts and alarm flags. The fix would
>   presumably be to add a call to pcf2123_reset in the init, but...
> 

It recommends doing a software reset after power-on in that case, it
refers to the power-on of the RTC, not the platform. You shouldn't do a
software reset as this will break time keeping, the offset and reading
alarms that may have been starting the platform.

> * pcf2123_reset stops the RTC for no apparent reason. Result is that the
>   time is invalid after a call to pcf2123_reset, which requires the time
>   to be set again manually. The fix would be to delete the stop commands.
> 

Using the software reset will render the time invalid anyway as this
will set the OS bit in the seconds register (see Table 7.
Register reset values).

> * 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].

> Not sure if I'll fix these issues right now. Also this RTC does have the 
> ability to do periodic interrupts but the feature has not been implemented
> in this driver. So I'll leave uie_unsupported set for now.
> 

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