.../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(-)
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
On Mon, Jan 20, 2025 at 03:41:45PM +0100, Johan Hovold 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. 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
On Sun, Jan 26, 2025 at 4:20 PM Rob Herring <robh@kernel.org> wrote: > > On Mon, Jan 20, 2025 at 03:41:45PM +0100, Johan Hovold 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. > > 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 BR, -R > OTOH, it's one property, meh. > > Rob >
On Mon, Apr 21, 2025 at 07:36:28AM -0700, Rob Clark wrote: > On Sun, Jan 26, 2025 at 4:20 PM Rob Herring <robh@kernel.org> wrote: > > On Mon, Jan 20, 2025 at 03:41:45PM +0100, Johan Hovold 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. > > > 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? > > 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). Hmm. It seems dropping that property for v2 under the assumption that efivars would be available at module init time (since the driver can only be built-in) was a mistake. I see now that the current qcom efivars driver does not probe until module init time itself, but even if we change that the scm driver depends on an interconnect driver which can be built as a module... > Hacking up rtc-pm8xxx to return -EPROBE_DEFER "fixes" it > > OTOH, it's one property, meh. I guess we need that property on these platforms as I had initially concluded after all. As with the rest of this driver, hopefully all of this goes away for future Qualcomm platforms once they fix their UEFI implementation so that the time service can be used directly. Johan
[ +CC: Ard and Maximilian ] Hi Rob, and sorry about the late follow up on this. I had to find some time to think more about this so it ended up on the back burner. On Sun, Jan 26, 2025 at 06:20:26PM -0600, Rob Herring wrote: > On Mon, Jan 20, 2025 at 03:41:45PM +0100, Johan Hovold 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. > > 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. You're right, I should have mentioned this in the commit message and linked to the RFC discussion directly: https://lore.kernel.org/lkml/Y9qO0yQ7oLux2L9n@hovoldconsulting.com/ > 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? Yeah, there are more things that expect EFI variables during early boot, including systemd. In fact, that is the reason why the Qualcomm efivars implementation can currently only be built in: https://lore.kernel.org/lkml/ZJ11H8btBhvCx9gD@hovoldconsulting.com/ The variable service is supposed to be a runtime service that is available when drivers probe (module init), so I think it's reasonably to simple refuse further modular efivars implementation (we already have another from Google). Since allowing the Qualcomm implementation to be modular now would regress user space it seems at least that one will need to stay built-in indefinitely. And again, hopefully all of this goes away (for future platforms) once Qualcomm fix their UEFI implementation so that the UEFI time and variable services can be used directly. I've dropped the DT property for v2. Johan
Hi, On Mon, Jan 20, 2025 at 03:41:45PM +0100, Johan Hovold 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. I've been using this series for the last few days and it seems to work well. Tested-by: Sebastian Reichel <sre@kernel.org> # Lenovo T14s Gen6 Greetings, -- Sebastian
On 20.01.2025 3:41 PM, Johan Hovold 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 Thanks for working on this! Konrad
On Tue, 21 Jan 2025 at 01:14, 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. Looks good on a Surface Laptop 7 / romulus: Tested-by: Joel Stanley <joel@jms.id.au> [ 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) Cheers, Joel
On Tue, Jan 21, 2025 at 02:18:21PM +1030, Joel Stanley wrote: > On Tue, 21 Jan 2025 at 01:14, 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. > > Looks good on a Surface Laptop 7 / romulus: > > Tested-by: Joel Stanley <joel@jms.id.au> > > [ 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) Thanks for testing. Johan
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>
On Mon, Jan 20, 2025 at 03:19:33PM -0600, Steev Klimaszewski wrote: > 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> Thanks for testing. Johan
© 2016 - 2025 Red Hat, Inc.