Re: [PATCH] MIPS: Loongson64: Add read_persistent_clock64()

[Tiezhu Yang <yangtiezhu@xxxxxxxxxxx>]

On 11/13/2020 10:21 AM, Jiaxun Yang wrote:
> 在 2020/11/12 20:03, Tiezhu Yang 写道:
> > On 11/12/2020 06:09 PM, Jiaxun Yang wrote:
> > > 在 2020/11/12 18:04, Jiaxun Yang 写道:
> > > > Hi Tiezhu,
> > > >
> > > > 在 2020/11/12 16:29, Tiezhu Yang 写道:
> > > > > Add read_persistent_clock64() to read the time from the battery 
> > > > > backed
> > > > > persistent clock. With this patch, we can fix the wrong time issue 
> > > > > due
> > > > > to the system clock is not consistent with hardware clock after 
> > > > > resume
> > > > > from sleep state S3 (suspend to RAM), at the same time, the system 
> > > > > time
> > > > > can be right instead of "Thu Jan 1 08:00:00 CST 1970" without rtc 
> > > > > driver.
> > > > >
> > > > > start_kernel()
> > > > >    timekeeping_init()
> > > > >      read_persistent_wall_and_boot_offset()
> > > > >        read_persistent_clock64()
> > > > >
> > > > > timekeeping_resume()
> > > > >    read_persistent_clock64()
> > > > >
> > > > > timekeeping_suspend()
> > > > >    read_persistent_clock64()
> > > >
> > > > It is highly discoraged to do anything with bridgetype, which isn't 
> > > > probed via
> > > > devicetree.
> > > >
> > > > Please check if you can deal with that inside RTC framework, or 
> > > > make it as
> > > > a part of RTC driver (e.g. set up a callback).
> > > >
> > > > Also you should submit RTC driver at first if you intend to 
> > > > complete LS7A support.
> > >
> > > Oops,
> > > Just dig it deeper, I guess simply select RTC_HCTOSYS would solve 
> > > the issue.
> > > We're trying very hard to decouple all the drivers and conponents,
> > > DeviceTree for all!
> >
> > +cc WANG Xuerui <git@xxxxxxxxxx>
> >
> > Hi Jiaxun,
> >
> > Thanks for your reply.
> >
> > Xuerui has already submitted the patch of LS7A rtc driver [1],
> > but not yet been merged into the mainline kernel, I discussed
> > with him early today.
> >
> > Do you mean that read_persistent_clock64() can call the function
> > like rtc_read_time() defined in rtc driver?
>
> I do think select RTC_HCTOSYS after getting RTC driver applied can help.

Yes, I agree.

> What's your point to have read_persistent_clock64 for Loongson64?

(1) Currently, the LS7A RTC driver has not been merged into the
mainline kernel, read_persistent_clock64() is useful in the following
call path:

start_kernel()
   timekeeping_init()
     read_persistent_wall_and_boot_offset()
       read_persistent_clock64()

(2) When the LS7A RTC driver is merged into the mainline kernel
some time later, if RTC_HCTOSYS and RTC_DRV_LS2X are not set,
read_persistent_clock64() is also useful unless RTC_HCTOSYS
and RTC_DRV_LS2X are set by default in Kconfig instead of
loongson3_defconfig.

So I think read_persistent_clock64() looks like a backup function.

> Thanks
>
> - Jiaxun
>
> > Thanks,
> > Tiezhu
> >
> > [1] 
> > https://patchwork.kernel.org/project/linux-mips/patch/20200923075845.360974-2-git@xxxxxxxxxx/
> >
> > > > Thanks.
> > > >
> > > > - Jiaxun
> > > >
> > > > > Signed-off-by: Yinglu Yang <yangyinglu@xxxxxxxxxxx>
> > > > > Signed-off-by: Tiezhu Yang <yangtiezhu@xxxxxxxxxxx>
> > > > > ---