arm64: dts: qcom: x1e80100: enable rtc

From: Jonathan Marek <[email protected]>

Qualcomm x1e80100 firmware sets the ownership of the RTC alarm to ADSP. Thus writing to RTC alarm registers and receiving alarm interrupts is not possible.

Add a qcom,no-alarm flag to support RTC on this platform.

Signed-off-by Jonathan Marek <[email protected]>

Link https://lore.kernel.org/r/[email protected] open here

[ johan: move vendor property; use boolean; reword description ]

Signed-off-by Johan Hovold <[email protected]>

---
 Documentation/devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml | 5 +++++
 1 file changed, 5 insertions(+)

On many Qualcomm platforms the PMIC RTC control and time registers are read-only so that the RTC time can not be updated. Instead an offset needs be stored in some machine-specific non-volatile memory, which the driver can take into account.

Add support for storing a 32-bit offset from the GPS time epoch in a UEFI variable so that the RTC time can be set on such platforms.

Quoted 5 lines · Johan Hovold

On many Qualcomm platforms the PMIC RTC control and time registers are read-only so that the RTC time can not be updated. Instead an offset needs be stored in some machine-specific non-volatile memory, which the driver can take into account.

Quoted 3 lines

Add support for storing a 32-bit offset from the GPS time epoch in a UEFI variable so that the RTC time can be set on such platforms.

Why can't the UEFI runtime services be used here ?
In short, why is drivers/rtc/rtc-efi.c not working or no attempts are made to check if that works just fine ?

Quoted 10 lines · Sudeep Holla

Quoted 8 lines · Johan Hovold

On many Qualcomm platforms the PMIC RTC control and time registers are read-only so that the RTC time can not be updated. Instead an offset needs be stored in some machine-specific non-volatile memory, which the driver can take into account.

Add support for storing a 32-bit offset from the GPS time epoch in a UEFI variable so that the RTC time can be set on such platforms.

Quoted 3 lines

Why can't the UEFI runtime services be used here ?
In short, why is drivers/rtc/rtc-efi.c not working or no attempts are made to check if that works just fine ?

As I alluded to in the commit message, the Qualcomm UEFI implementation

is broken and the runtime services cannot be accessed during runtime...

That's why we can't use rtc-efi and why we have a dedicated efivars implementation that talks directly to the firmware using a custom protocol:

	https://lore.kernel.org/all/[email protected]/

Quoted 11 lines · Johan Hovold

Quoted 10 lines · Sudeep Holla

Quoted 8 lines · Johan Hovold

On many Qualcomm platforms the PMIC RTC control and time registers are read-only so that the RTC time can not be updated. Instead an offset needs be stored in some machine-specific non-volatile memory, which the driver can take into account.

Add support for storing a 32-bit offset from the GPS time epoch in a UEFI variable so that the RTC time can be set on such platforms.

Quoted 5 lines

Quoted 3 lines

Why can't the UEFI runtime services be used here ?
In short, why is drivers/rtc/rtc-efi.c not working or no attempts are made to check if that works just fine ?

As I alluded to in the commit message, the Qualcomm UEFI implementation

s/commit message/cover letter/

On many Qualcomm platforms the PMIC RTC control and time registers are read-only so that the RTC time can not be updated. Instead an offset needs be stored in some machine-specific non-volatile memory, which the driver can take into account.

Add support for storing a 32-bit offset from the GPS time epoch in a UEFI variable so that the RTC time can be set on such platforms.

The UEFI variable is

            882f8c2b-9646-435f-8de5-f208ff80c1bd-RTCInfo

and holds a 12-byte structure where the first four bytes is a GPS time offset in little-endian byte order.

Note that this format is not arbitrary as the variable is shared with the UEFI firmware (and Windows).

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/rtc/rtc-pm8xxx.c | 125 +++++++++++++++++++++++++++++++++++++--
 include/linux/rtc.h      |   1 +
 2 files changed, 121 insertions(+), 5 deletions(-)

drivers/rtc/rtc-pm8xxx.c +120 -5

index 2f32187ecc8d..be1983bf35cd 100644
@@ -5,6 +5,7 @@
  * Copyright (c) 2010-2011, Code Aurora Forum. All rights reserved.
  * Copyright (c) 2023, Linaro Limited
  */
+#include <linux/efi.h>
 #include <linux/of.h>
 #include <linux/module.h>
 #include <linux/nvmem-consumer.h>

@@ -16,9 +17,10 @@
 #include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
-
 #include <linux/unaligned.h>
 
+#include <asm/byteorder.h>
+
 /* RTC_CTRL register bit fields */
 #define PM8xxx_RTC_ENABLE		BIT(7)
 #define PM8xxx_RTC_ALARM_CLEAR		BIT(0)

@@ -46,14 +48,21 @@ struct pm8xxx_rtc_regs {
 	unsigned int alarm_en;
 };
 
+struct qcom_uefi_rtc_info {
+	__le32	offset_gps;
+	u8	reserved[8];
+} __packed;
+
 /**
  * struct pm8xxx_rtc -  RTC driver internal structure
  * @rtc:		RTC device
  * @regmap:		regmap used to access registers
  * @allow_set_time:	whether the time can be set
+ * @use_uefi:		use UEFI variable as fallback for offset
  * @alarm_irq:		alarm irq number
  * @regs:		register description
  * @dev:		device structure
+ * @rtc_info:		qcom uefi rtc-info structure
  * @nvmem_cell:		nvmem cell for offset
  * @offset:		offset from epoch in seconds
  */

@@ -61,13 +70,101 @@ struct pm8xxx_rtc {
 	struct rtc_device *rtc;
 	struct regmap *regmap;
 	bool allow_set_time;
+	bool use_uefi;
 	int alarm_irq;
 	const struct pm8xxx_rtc_regs *regs;
 	struct device *dev;
+	struct qcom_uefi_rtc_info rtc_info;
 	struct nvmem_cell *nvmem_cell;
 	u32 offset;
 };
 
+#ifdef CONFIG_EFI
+
+MODULE_IMPORT_NS("EFIVAR");
+
+#define QCOM_UEFI_NAME	L"RTCInfo"
+#define QCOM_UEFI_GUID	EFI_GUID(0x882f8c2b, 0x9646, 0x435f, \
+				 0x8d, 0xe5, 0xf2, 0x08, 0xff, 0x80, 0xc1, 0xbd)
+#define QCOM_UEFI_ATTRS	(EFI_VARIABLE_NON_VOLATILE | \
+			 EFI_VARIABLE_BOOTSERVICE_ACCESS | \
+			 EFI_VARIABLE_RUNTIME_ACCESS)
+
+static int pm8xxx_rtc_read_uefi_offset(struct pm8xxx_rtc *rtc_dd)
+{
+	struct qcom_uefi_rtc_info *rtc_info = &rtc_dd->rtc_info;
+	unsigned long size = sizeof(*rtc_info);
+	struct device *dev = rtc_dd->dev;
+	efi_status_t status;
+	u32 offset_gps;
+	int rc;
+
+	rc = efivar_lock();
+	if (rc)
+		return rc;
+
+	status = efivar_get_variable(QCOM_UEFI_NAME, &QCOM_UEFI_GUID, NULL,
+				     &size, rtc_info);
+	efivar_unlock();
+
+	if (status != EFI_SUCCESS) {
+		dev_dbg(dev, "failed to read UEFI offset: %lu\n", status);
+		return efi_status_to_err(status);
+	}
+
+	if (size != sizeof(*rtc_info)) {
+		dev_dbg(dev, "unexpected UEFI structure size %lu\n", size);
+		return -EINVAL;
+	}
+
+	dev_dbg(dev, "uefi_rtc_info = %*ph\n", (int)size, rtc_info);
+
+	/* Convert from GPS to Unix time offset */
+	offset_gps = le32_to_cpu(rtc_info->offset_gps);
+	rtc_dd->offset = offset_gps + (u32)RTC_TIMESTAMP_EPOCH_GPS;
+
+	return 0;
+}
+
+static int pm8xxx_rtc_write_uefi_offset(struct pm8xxx_rtc *rtc_dd, u32 offset)
+{
+	struct qcom_uefi_rtc_info *rtc_info = &rtc_dd->rtc_info;
+	unsigned long size = sizeof(*rtc_info);
+	struct device *dev = rtc_dd->dev;
+	efi_status_t status;
+	u32 offset_gps;
+
+	/* Convert from Unix to GPS time offset */
+	offset_gps = offset - (u32)RTC_TIMESTAMP_EPOCH_GPS;
+
+	rtc_info->offset_gps = cpu_to_le32(offset_gps);
+
+	dev_dbg(dev, "efi_rtc_info = %*ph\n", (int)size, rtc_info);
+
+	status = efivar_set_variable(QCOM_UEFI_NAME, &QCOM_UEFI_GUID,
+				     QCOM_UEFI_ATTRS, size, rtc_info);
+	if (status != EFI_SUCCESS) {
+		dev_dbg(dev, "failed to write UEFI offset: %lx\n", status);
+		return efi_status_to_err(status);
+	}
+
+	return 0;
+}
+
+#else	/* CONFIG_EFI */
+
+static int pm8xxx_rtc_read_uefi_offset(struct pm8xxx_rtc *rtc_dd)
+{
+	return -ENODEV;
+}
+
+static int pm8xxx_rtc_write_uefi_offset(struct pm8xxx_rtc *rtc_dd, u32 offset)
+{
+	return -ENODEV;
+}
+
+#endif	/* CONFIG_EFI */
+
 static int pm8xxx_rtc_read_nvmem_offset(struct pm8xxx_rtc *rtc_dd)
 {
 	size_t len;

@@ -112,10 +209,13 @@ static int pm8xxx_rtc_write_nvmem_offset(struct pm8xxx_rtc *rtc_dd, u32 offset)
 
 static int pm8xxx_rtc_read_offset(struct pm8xxx_rtc *rtc_dd)
 {
-	if (!rtc_dd->nvmem_cell)
+	if (!rtc_dd->nvmem_cell && !rtc_dd->use_uefi)
 		return 0;
 
-	return pm8xxx_rtc_read_nvmem_offset(rtc_dd);
+	if (rtc_dd->nvmem_cell)
+		return pm8xxx_rtc_read_nvmem_offset(rtc_dd);
+	else
+		return pm8xxx_rtc_read_uefi_offset(rtc_dd);
 }
 
 static int pm8xxx_rtc_read_raw(struct pm8xxx_rtc *rtc_dd, u32 *secs)

@@ -155,7 +255,7 @@ static int pm8xxx_rtc_update_offset(struct pm8xxx_rtc *rtc_dd, u32 secs)
 	u32 offset;
 	int rc;
 
-	if (!rtc_dd->nvmem_cell)
+	if (!rtc_dd->nvmem_cell && !rtc_dd->use_uefi)
 		return -ENODEV;
 
 	rc = pm8xxx_rtc_read_raw(rtc_dd, &raw_secs);

@@ -167,7 +267,11 @@ static int pm8xxx_rtc_update_offset(struct pm8xxx_rtc *rtc_dd, u32 secs)
 	if (offset == rtc_dd->offset)
 		return 0;
 
-	rc = pm8xxx_rtc_write_nvmem_offset(rtc_dd, offset);
+	if (rtc_dd->nvmem_cell)
+		rc = pm8xxx_rtc_write_nvmem_offset(rtc_dd, offset);
+	else
+		rc = pm8xxx_rtc_write_uefi_offset(rtc_dd, offset);
+
 	if (rc)
 		return rc;
 

@@ -486,6 +590,17 @@ static int pm8xxx_rtc_probe(struct platform_device *pdev)
 		if (rc != -ENOENT)
 			return rc;
 		rtc_dd->nvmem_cell = NULL;
+
+		/* Use UEFI storage as fallback if available */
+		rtc_dd->use_uefi = of_property_read_bool(pdev->dev.of_node,
+							 "qcom,uefi-rtc-info");
+	}
+
+	if (rtc_dd->use_uefi && !efivar_is_available()) {
+		if (IS_ENABLED(CONFIG_EFI))
+			return -EPROBE_DEFER;
+		dev_warn(&pdev->dev, "efivars not available\n");
+		rtc_dd->use_uefi = false;
 	}
 
 	rtc_dd->regs = match->data;

include/linux/rtc.h +1 -0

index 3f4d315aaec9..95da051fb155 100644
@@ -170,6 +170,7 @@ struct rtc_device {
 /* useful timestamps */
 #define RTC_TIMESTAMP_BEGIN_0000	-62167219200ULL /* 0000-01-01 00:00:00 */
 #define RTC_TIMESTAMP_BEGIN_1900	-2208988800LL /* 1900-01-01 00:00:00 */
+#define RTC_TIMESTAMP_EPOCH_GPS		315964800LL /* 1980-01-06 00:00:00 */
 #define RTC_TIMESTAMP_BEGIN_2000	946684800LL /* 2000-01-01 00:00:00 */
 #define RTC_TIMESTAMP_END_2063		2966371199LL /* 2063-12-31 23:59:59 */
 #define RTC_TIMESTAMP_END_2079		3471292799LL /* 2079-12-31 23:59:59 */

Signature · 1 line

2.45.2

Quoted 7 lines · Johan Hovold

On many Qualcomm platforms the PMIC RTC control and time registers are read-only so that the RTC time can not be updated. Instead an offset needs be stored in some machine-specific non-volatile memory, which the driver can take into account.

Add support for storing a 32-bit offset from the GPS time epoch in a UEFI variable so that the RTC time can be set on such platforms.

Thanks for the great work Johan, seems to work nicely here!

I think you might need to add MODULE_IMPORT_NS(EFIVAR) for the efivar_* API calls you added.

drivers/rtc/rtc-pm8xxx.c struct pm8xxx_rtc { · line 84

Quoted 3 lines

drivers/rtc/rtc-pm8xxx.c +3 -0

+#ifdef CONFIG_EFI
+
+MODULE_IMPORT_NS("EFIVAR");

On many Qualcomm platforms the PMIC RTC control and time registers are read-only so that the RTC time can not be updated. Instead an offset needs be stored in some machine-specific non-volatile memory, which the driver can take into account.

On machines like the Lenovo ThinkPad X14s the PMIC RTC drifts about one

second every 3.5 hours, something which leads to repeated updates of the offset when NTP synchronisation is enabled.

Reduce wear of the underlying flash storage (used for UEFI variables) by deferring writes until shutdown in case they appear to be due to clock drift.

As an example, allowing the on-flash offset to differ up to 30 s reduces

the number of writes on the X13s during a ten day session with the machine not suspending for more than four days in a row from up to 68 writes (every 3.5 h) to at most two (boot and shutdown).

On many Qualcomm platforms the PMIC RTC control and time registers are read-only so that the RTC time can not be updated. Instead an offset needs be stored in some machine-specific non-volatile memory, which a driver can take into account.

Switch to using the Qualcomm specific UEFI variable that is used by the UEFI firmware (and Windows) to store the RTC offset.

This specifically means that the RTC time will be synchronised between the UEFI firmware setup (or UEFI shell), Windows and Linux.

Note however that Windows stores the RTC time in local time by default, while Linux typically uses UTC (i.e. as on X86).

Signed-off-by Johan Hovold <[email protected]>

---
  .../boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts   | 11 +----------
  1 file changed, 1 insertion(+), 10 deletions(-)

arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts +1 -3

index 38d911992475..66c39765225f 100644
@@ -1090,20 +1090,11 @@ &pmk8280_pon_resin {
  };
 
  &pmk8280_rtc {
-	nvmem-cells = <&rtc_offset>;
-	nvmem-cell-names = "offset";
+	qcom,uefi-rtc-info;
 
  	status = "okay";
  };
 
-&pmk8280_sdam_6 {

On many Qualcomm platforms the PMIC RTC control and time registers are read-only so that the RTC time can not be updated. Instead an offset needs be stored in some machine-specific non-volatile memory, which a driver can take into account.

Switch to using the Qualcomm specific UEFI variable that is used by the UEFI firmware (and Windows) to store the RTC offset.

This specifically means that the RTC time will be synchronised between the UEFI firmware setup (or UEFI shell), Windows and Linux.

Note however that Windows stores the RTC time in local time by default, while Linux typically uses UTC (i.e. as on X86).

Signed-off-by Johan Hovold <[email protected]>

---

On many Qualcomm platforms the PMIC RTC control and time registers are read-only so that the RTC time can not be updated. Instead an offset needs be stored in some machine-specific non-volatile memory, which a driver can take into account.

On X1E based Windows on Arm machines the offset is stored in a Qualcomm specific UEFI variable.

Unlike on previous platforms the alarm registers are also unaccessible on X1E as they are owned by the ADSP.

Assume all X1E machines use similar firmware and enable the RTC in the PMIC dtsi for now.

Based on a patch by Jonathan Marek. [1]

Link https://lore.kernel.org/r/[email protected] # [1] open here

Signed-off-by Johan Hovold <[email protected]>

---
  arch/arm64/boot/dts/qcom/x1e80100-pmics.dtsi | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm64/boot/dts/qcom/x1e80100-pmics.dtsi +0 -0

index 5b54ee79f048..051fb3a304b9 100644

works nicely on SnapDragon Dev Kit X1E001DE. Thank you!

Tested-by Jens Glathe <[email protected]>

On many Qualcomm platforms the PMIC RTC control and time registers are read-only so that the RTC time can not be updated. Instead an offset needs be stored in some machine-specific non-volatile memory, which a driver can take into account.

On X1E based Windows on Arm machines the offset is stored in a Qualcomm specific UEFI variable.

Unlike on previous platforms the alarm registers are also unaccessible on X1E as they are owned by the ADSP.

Assume all X1E machines use similar firmware and enable the RTC in the PMIC dtsi for now.

Based on a patch by Jonathan Marek. [1]

Link https://lore.kernel.org/r/[email protected] # [1] open here

Signed-off-by Johan Hovold <[email protected]>

---

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/[email protected]/
[2] https://lore.kernel.org/lkml/[email protected]/
[3] https://lore.kernel.org/lkml/[email protected]/

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 <[email protected]>

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.

Quoted 4 lines · Johan Hovold

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.

Looks good on a Surface Laptop 7 / romulus:

Tested-by Joel Stanley <[email protected]>

[    0.407844] rtc-pm8xxx c42d000.spmi:pmic@0:rtc@6100: registered as rtc0
[    0.407876] rtc-pm8xxx c42d000.spmi:pmic@0:rtc@6100: setting system

clock to 2025-01-21T03:34:06 UTC (1737430446)

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

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.

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.

Quoted 32 lines · Johan Hovold

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.

Quoted 5 lines

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.

This information would be useful in the binding commit...

Seems like something I would say, but this is v1 and I have no memory of discussing this.

I would also say probe ordering is not a DT problem,
but sounds like an OS problem. Aren't there other things needing EFI variables earlyish too? Do you really want to have to update the DT to enable this?

Quoted 32 lines · Johan Hovold

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.

Quoted 5 lines

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.

This information would be useful in the binding commit...

Seems like something I would say, but this is v1 and I have no memory of discussing this. I would also say probe ordering is not a DT problem, but sounds like an OS problem. Aren't there other things needing EFI variables earlyish too? Do you really want to have to update the DT to enable this?

OTOH, it's one property, meh.

Rob

Quoted 11 lines · Rob Herring

Quoted 9 lines · Johan Hovold

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.

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.

Quoted 7 lines

This information would be useful in the binding commit...

Seems like something I would say, but this is v1 and I have no memory of discussing this. I would also say probe ordering is not a DT problem, but sounds like an OS problem. Aren't there other things needing EFI variables earlyish too? Do you really want to have to update the DT to enable this?

I was debugging why RTC offset was not working properly for me, and eventually realized it was exactly this problem (efivars not avail when rtc probes).

Hacking up rtc-pm8xxx to return -EPROBE_DEFER "fixes" it

Quoted 1 line

OTOH, it's one property, meh.