Re: [PATCH 1/2] rtc: pcf2127: Disable Power-On Reset Override

[Philipp Rosenberger <p.rosenberger@kunbus.com>]

Hi Uwe,

On 12.01.21 20:26, Uwe Kleine-König wrote:
> Hello,
> 
> On Mon, Jan 04, 2021 at 05:19:09PM +0100, Philipp Rosenberger wrote:
>> If the PCF2127/2129 has lost all power and is then powered again it goes
>> into "Power-On Reset Override" mode. In this mode the RTC seems to work
>> fine. Also the watchdog can be configured. The watchdog timer counts as
>> expected and the WDTF (watchdog timer flag) gets set. But no interrupt
>> is generated on the INT pin. The same applies to the alarm function.
>>
>> The POR_OVRD bit on the Control_1 register must be cleared first. In
>> some cases the bootloader or firmware might have done this already. But
>> we clear the bit nevertheless to guarantee correct behavior the
>> watchdog and alarm function.
> 
> I don't understand this. The reference manual tells us about this bit:
> 
> | The POR duration is directly related to the crystal oscillator
> | start-up time. Due to the long start-up times experienced by these
> | types of circuits, a mechanism has been built in to disable the POR
> | and therefore speed up the on-board test of the device.
> | The setting of the PORO mode requires that POR_OVRD in register
> | Control_1 is set logic 1 and that the signals at the interface pins
> | SDA/CE and SCL are toggled as illustrated in Figure 18.
> 
> So this means that with the bit set (i.e. with this patch added) after a
> power-on the oscillator isn't properly reset. This means the chip might
> not work correctly, right? Does "speed up the on-board test" suggest,
> this is a feature that is to be used while testing the chip and not for
> production use? You missed to ensure that the requested toggling is
> done. Did you test how much time this actually saves? I doubt it is
> worth to trade correct operation for quicker startup time is the thing
> we want here.
> 
> If you still think this is a good idea I guess you need a much better
> commit log (and code comment).

Yes I guess the commit log and the comment are not good enough. I took 
me a long time to find what was wrong with my setup until I realized the 
the PORO was the problem. I find the description in the manual not very 
clear. But from my tests and from the description in Table 7 Bit 3 it is 
pretty clear that the PORO bit should not be set during normal operation.

| Power-On Reset Override (PORO) facility disabled;
|  *set logic 0 for normal operation*

My tests have shown, that as long as the bit is set not interrupts are 
generated on the interrupt pin. So the POR_OVRD bit needs to be cleared 
for normal operation. At least that is my understanding.

If nobody disagrees with my assumption I would try to reword the commit 
log and the comment for a v2.

> Best regards
> Uwe
> 

Best regards,
Philipp