Re: Question about PCF85063A invalid state

[Phil Elwell <phil@raspberrypi.com>]

Alexandre,

On 13/10/2021 21:14, Alexandre Belloni wrote:
> Hello,
> 
> On 13/10/2021 10:48:48+0100, Phil Elwell wrote:
>> Hi,
>>
>> We're using the PCF85063A on our Compute Module 4 I/O board, and a number of
>> users have experienced problems with invalid state after inserting a backup
>> battery. So far the problem has been confined to EXT_TEST (bit 7 of CTRL_1)
>> being set, but there are a number of registers for which the driver only
>> modifies a subset of the bits.
>>
>> I can think of a number of ways to clear this invalid state, none of which
>> are difficult - force a software reset whenever certain bits of certain
>> registers have certain values, always specifically force some bits to known
>> values, etc. -
>> but are there any preferences or pitfalls to be aware of?
>>
> 
> This is a very difficult topic because contrary to most of the other
> IPs, the RTC is still running while Linux is not and it is expected that
> it will retain its configuration across reboots (well, this is exactly
> what the RTC is for).
> 
> This means that you may have some code (bootloader, firmware) running
> before Linux configuring the RTC and so setting or clearing bits in
> registers that are never touched by Linux. So, it is not possible to go
> and change random bits because they don't have the default value or the
> value we expect. Instead it is usually necessary to provide a way to
> set those bits.
> 
> However, for EXT_TEST, I would assume the RTC lost its time once it is
> set so I would treat it just like OS: if it is set, then leave it that
> way until .set_time is called and return -EINVAL in .read_time.
> 
> I didn't try but if OS is always set by the RTC when EXT_TEST is set,
> then maybe we don't need to check in .read_time.

I think your suggestion can be summarised as:

The existing code probably already detects EXT_TEST because it will cause
the OS bit to be set. All that is required is to clear the EXT_TEST bit
as part of the usual .set_time handler.

I will do it that way.

Many thanks,

Phil