Re: [PATCH 3/5] rtc: jz4740: Reset scratchpad register on power loss

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





Le mar., avril 19 2022 at 22:00:32 +0200, Alexandre Belloni <alexandre.belloni@xxxxxxxxxxx> a écrit :
On 19/04/2022 20:48:54+0100, Paul Cercueil wrote:
 Hi Alexandre,

 Le mar., avril 19 2022 at 21:35:35 +0200, Alexandre Belloni
 <alexandre.belloni@xxxxxxxxxxx> a écrit :
 > On 18/04/2022 19:49:31+0100, Paul Cercueil wrote:
> > On power loss, reading the RTC value would fail as the scratchpad
 > > lost
 > >  its magic value, until the hardware clock was set once again.
 > >
 > >  To avoid that, reset the RTC value to Epoch in the probe if we
 > > detect
 > >  that the scratchpad lost its magic value.
 > >
 > >  Signed-off-by: Paul Cercueil <paul@xxxxxxxxxxxxxxx>
 > >  ---
 > >   drivers/rtc/rtc-jz4740.c | 24 +++++++++++++++++++++---
 > >   1 file changed, 21 insertions(+), 3 deletions(-)
 > >
> > diff --git a/drivers/rtc/rtc-jz4740.c b/drivers/rtc/rtc-jz4740.c
 > >  index 119baf168b32..aac5f68bf626 100644
 > >  --- a/drivers/rtc/rtc-jz4740.c
 > >  +++ b/drivers/rtc/rtc-jz4740.c
 > >  @@ -42,6 +42,9 @@
 > >   /* Magic value to enable writes on jz4780 */
 > >   #define JZ_RTC_WENR_MAGIC	0xA55A
 > >
 > >  +/* Value written to the scratchpad to detect power losses */
 > >  +#define JZ_RTC_SCRATCHPAD_MAGIC	0x12345678
 > >  +
 > >   #define JZ_RTC_WAKEUP_FILTER_MASK	0x0000FFE0
 > >   #define JZ_RTC_RESET_COUNTER_MASK	0x00000FE0
 > >
> > @@ -134,10 +137,11 @@ static int jz4740_rtc_ctrl_set_bits(struct
 > > jz4740_rtc *rtc, uint32_t mask,
 > >   static int jz4740_rtc_read_time(struct device *dev, struct
 > > rtc_time *time)
 > >   {
 > >   	struct jz4740_rtc *rtc = dev_get_drvdata(dev);
 > >  -	uint32_t secs, secs2;
 > >  +	uint32_t secs, secs2, magic;
 > >   	int timeout = 5;
 > >
> > - if (jz4740_rtc_reg_read(rtc, JZ_REG_RTC_SCRATCHPAD) != 0x12345678)
 > >  +	magic = jz4740_rtc_reg_read(rtc, JZ_REG_RTC_SCRATCHPAD);
 > >  +	if (magic != JZ_RTC_SCRATCHPAD_MAGIC)
 > >   		return -EINVAL;
 > >
> > /* If the seconds register is read while it is updated, it can
 > > contain a
> > @@ -169,7 +173,8 @@ static int jz4740_rtc_set_time(struct device
 > > *dev, struct rtc_time *time)
 > >   	if (ret)
 > >   		return ret;
 > >
 > >  -	return jz4740_rtc_reg_write(rtc, JZ_REG_RTC_SCRATCHPAD,
 > > 0x12345678);
 > >  +	return jz4740_rtc_reg_write(rtc, JZ_REG_RTC_SCRATCHPAD,
 > >  +				    JZ_RTC_SCRATCHPAD_MAGIC);
 > >   }
 > >
 > >   static int jz4740_rtc_read_alarm(struct device *dev, struct
 > > rtc_wkalrm *alrm)
 > >  @@ -307,6 +312,7 @@ static int jz4740_rtc_probe(struct
 > > platform_device *pdev)
 > >   	struct jz4740_rtc *rtc;
 > >   	unsigned long rate;
 > >   	struct clk *clk;
 > >  +	uint32_t magic;
 > >   	int ret, irq;
 > >
 > >   	rtc = devm_kzalloc(dev, sizeof(*rtc), GFP_KERNEL);
 > >  @@ -369,6 +375,18 @@ static int jz4740_rtc_probe(struct
 > > platform_device *pdev)
 > >   	/* Each 1 Hz pulse should happen after (rate) ticks */
 > >   	jz4740_rtc_reg_write(rtc, JZ_REG_RTC_REGULATOR, rate - 1);
 > >
 > >  +	magic = jz4740_rtc_reg_read(rtc, JZ_REG_RTC_SCRATCHPAD);
 > >  +	if (magic != JZ_RTC_SCRATCHPAD_MAGIC) {
 > >  +		/*
 > >  +		 * If the scratchpad doesn't hold our magic value, then a
 > >  +		 * power loss occurred. Reset to Epoch.
 > >  +		 */
 > >  +		struct rtc_time time;
 > >  +
 > >  +		rtc_time64_to_tm(0, &time);
 > >  +		jz4740_rtc_set_time(dev, &time);
 >
> Don't do that, this defeats the purpose of detecting when the power is > lost. Returning a known bogus time is the worst thing you can do here.

 So what is the best thing to do then?


Well, -EINVAL is returned when the time is invalid, this should be
enough. I'm not actually sure what is the issue you are trying to fix
here.

htop fails to start and tells me:
"No btime in /proc/stat: No such file or directory"

until the date is reset. So I was assuming it was a case of the jz4740 driver not being correct and breaking userspace.

Cheers,
-Paul



 Cheers,
 -Paul

 > >  +	}
 > >  +
 > >   	ret = devm_rtc_register_device(rtc->rtc);
 > >   	if (ret)
 > >   		return ret;
 > >  --
 > >  2.35.1
 > >
 >
 > --
 > Alexandre Belloni, co-owner and COO, Bootlin
 > Embedded Linux and Kernel engineering
 > https://bootlin.com



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






[Index of Archives]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux