Messages in this thread | | | Subject | Re: [PATCH v2 2/2] rtc: pcf2127: Run a OTP refresh if not done before | From | Philipp Rosenberger <> | Date | Thu, 14 Jan 2021 12:18:22 +0100 |
| |
On 14.01.21 12:11, Alexandre Belloni wrote: > On 14/01/2021 11:30:37+0100, Philipp Rosenberger wrote: >>>> + ret = regmap_set_bits(pcf2127->regmap, PCF2127_REG_CLKOUT, >>>> + PCF2127_BIT_CLKOUT_OTPR); >>>> + if (ret < 0) { >>>> + dev_err(dev, "%s: OTP refresh (clkout_ctrl) failed.\n", __func__); >>> >>> Please drop this error message. >> >> If I return from the probe with an error, shouldn't there be an error >> message? Or should I ignore the problem at all and don't return from the >> probe? > > You can return from probe without an error message. > >> >>> >>>> + return ret; >>>> + } >>>> + msleep(100); >>> >>> Maybe this should be done just before setting the time. Or if you want >>> to keep it in probe, then you could optimise by not waiting but ensuring >>> the time between pcf2127_probe and the first pcf2127_rtc_set_time is >>> more than 100ms. >>> >> >> Doing it just before setting the time might be not the best way. The >> watchdog might be used before the OTPR is done. >> >> From the PCF2129 manual: >> | The OTP refresh (see Section 8.3.2 on page 13) should ideally be >> | executed as the first instruction after start-up and also after a >> | reset due to an oscillator stop. >> >> As I see it this should be done before setting up the watchdog as well. So >> sleeping if the OTPR wasn't done before might be the most viable solution. >> So I would check the OTPR and only if the OTPR is not set starting an OTPR >> and then sleep 100ms. >> > > Indeed, the remaining question is whether you should test OTPR or OSF. > OSF states: "oscillator has stopped and chip reset has occurred since > flag was last cleared" if OTPR is always 0 when OSF is 1, then OTPR is > probably enough.
The datasheet is unclear about that. And I thought about that as well. The OSF flag is and should only reset when the time is set. But if I reboot or reload the driver without setting the time the OTRP would be rerun. So I thought it would be best to only rely on the OTPR bit. If someone has a better idea I'm open far that.
Best Regards, Philipp
| |