Re: [PATCH 0/7] arm64: dts: qcom: x1e80100: enable rtc

[Steev Klimaszewski <steev@kali.org>]

Hi Johan,

On Mon, Jan 20, 2025 at 8:43 AM Johan Hovold <johan+linaro@kernel.org> wrote:
>
> This series adds support for utilising the UEFI firmware RTC offset to
> the Qualcomm PMIC RTC driver and uses that to enable the RTC on all X
> Elite machines.
>
> Included is also a patch to switch the Lenovo ThinkPad X13s over to
> using the UEFI offset.
>
> The RTCs in many Qualcomm devices are effectively broken due to the time
> registers being read-only. Instead some other non-volatile memory can be
> used to store an offset which a driver can take into account. On Windows
> on Arm laptops, the UEFI firmware (and Windows) use a UEFI variable for
> storing such an offset.
>
> When RTC support for the X13s was added two years ago we did not yet
> have UEFI variable support for these machines in mainline and there were
> also some concerns regarding flash wear. [1] As not all Qualcomm
> platforms have UEFI firmware anyway, we instead opted to use a PMIC
> scratch register for storing the offset. [2]
>
> On the UEFI machines in question this is however arguable not correct
> as it means that the RTC time can differ between the UEFI firmware (and
> Windows) and Linux.
>
> Now that the (reverse engineered) UEFI variable implementation has been
> merged and thoroughly tested, let's switch to using that to store the
> RTC offset also on Linux. The flash wear concerns can be mitigated by
> deferring writes due to clock drift until shutdown.
>
> Note that this also avoids having to wait for months for Qualcomm to
> provide a free PMIC SDAM scratch register for X1E and future platforms,
> and specifically allows us to enable the RTC on X1E laptops today.
>
> Rob had some concerns about adding a DT property for indicating that a
> machine uses UEFI for storing the offset and suggested that the driver
> should probe for this instead. Unfortunately, this is easier said than
> done given that UEFI variable support itself is probed for and may not
> be available until after the RTC driver probes.
>
> Hopefully this all goes away (for future platforms) once Qualcomm fix
> their UEFI implementation so that the UEFI time (and variable) services
> can be used directly.
>
> Johan
>
>
> Changes since UEFI offset RFC [1]:
>  - clarify that UEFI variable format is not arbitrary (Alexandre)
>  - add missing use_uefi kernel doc
>  - use dev_dbg() instead of dev_err() (Alexandre)
>  - rename epoch define RTC_TIMESTAMP_EPOCH_GPS (Alexandre)
>  - mitigate flash wear by deferring writes due to clock drift until
>    shutdown
>
> Changes since Jonathan's X1E series v3 [3]:
>  - tweak qcom,no-alarm binding update (and drop Krystzof's Reviewed-by tag)
>  - drop no-alarm flag and restructure probe() to clear feature flag before
>    registering RTC
>  - use UEFI variable offset on X1E
>
> [1] https://lore.kernel.org/lkml/20230126142057.25715-1-johan+linaro@kernel.org/
> [2] https://lore.kernel.org/lkml/20230202155448.6715-1-johan+linaro@kernel.org/
> [3] https://lore.kernel.org/lkml/20241015004945.3676-1-jonathan@marek.ca/
>
>
> Johan Hovold (5):
>   dt-bindings: rtc: qcom-pm8xxx: add uefi-variable offset
>   rtc: pm8xxx: add support for uefi offset
>   rtc: pm8xxx: mitigate flash wear
>   arm64: dts: qcom: sc8280xp-x13s: switch to uefi rtc offset
>   arm64: dts: qcom: x1e80100: enable rtc
>
> Jonathan Marek (2):
>   dt-bindings: rtc: qcom-pm8xxx: document qcom,no-alarm flag
>   rtc: pm8xxx: implement qcom,no-alarm flag for non-HLOS owned alarm
>
>  .../bindings/rtc/qcom-pm8xxx-rtc.yaml         |  11 +
>  .../qcom/sc8280xp-lenovo-thinkpad-x13s.dts    |  11 +-
>  arch/arm64/boot/dts/qcom/x1e80100-pmics.dtsi  |   4 +-
>  drivers/rtc/rtc-pm8xxx.c                      | 194 +++++++++++++++---
>  include/linux/rtc.h                           |   1 +
>  5 files changed, 185 insertions(+), 36 deletions(-)
>
> --
> 2.45.2
>
>

Tested this on the Thinkpad X13s, as well as booting it into Windows
and verifying that it has the correct clock both ways, which it does.
Thank you!

Tested-by: Steev Klimaszewski <steev@kali.org>