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
