[RFC,18/24] dt-bindings: rtc: qcom-pm8xxx: add uefi-variable offset

Message ID 20230126142057.25715-19-johan+linaro@kernel.org
State New
Headers
Series rtc: pm8xxx: add support for setting time using nvmem |

Commit Message

Johan Hovold Jan. 26, 2023, 2:20 p.m. UTC
  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.

Add a 'qcom,uefi-rtc-info' boolean flag which indicates that the RTC
offset is stored in a Qualcomm specific UEFI variable so that the RTC
time can be updated 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.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 Documentation/devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml | 6 ++++++
 1 file changed, 6 insertions(+)
  

Comments

Rob Herring Jan. 30, 2023, 6:49 p.m. UTC | #1
On Thu, Jan 26, 2023 at 03:20:51PM +0100, Johan Hovold wrote:
> 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.
> 
> Add a 'qcom,uefi-rtc-info' boolean flag which indicates that the RTC
> offset is stored in a Qualcomm specific UEFI variable so that the RTC
> time can be updated 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.

Can't you just try to read the UEFI variable and use it if that 
succeeds?

I don't like this in DT because what if lots of devices start storing 
lots of things in vendor specific UEFI variables. It doesn't scale.

Rob
  
Johan Hovold Feb. 1, 2023, 4:09 p.m. UTC | #2
[ +CC: Ard ]

On Mon, Jan 30, 2023 at 12:49:44PM -0600, Rob Herring wrote:
> On Thu, Jan 26, 2023 at 03:20:51PM +0100, Johan Hovold wrote:
> > 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.
> > 
> > Add a 'qcom,uefi-rtc-info' boolean flag which indicates that the RTC
> > offset is stored in a Qualcomm specific UEFI variable so that the RTC
> > time can be updated 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.
> 
> Can't you just try to read the UEFI variable and use it if that 
> succeeds?

Generally, yes. The problem here is that this UEFI variable is not used
on all devices using these PMICs and I need a way to determine whether
to wait for the UEFI variables to become available or not (e.g. when
efivars support is built as module, yes, that's a thing now...).

> I don't like this in DT because what if lots of devices start storing 
> lots of things in vendor specific UEFI variables. It doesn't scale.

I hope we won't see that even if we already have some devices for x86
platforms storing MAC addresses and such in UEFI variables. They
currently access the UEFI firmware directly (i.e. not using the efivars
abstraction) and simply assume UEFI is always there.

With the Google SMI efivars implementation or the new Qualcomm SMC-based
one, we need a way to determine whether to wait for efivars to become
registered. For drivers where efivars is always needed we can just probe
defer, but in this case we should not wait unless the DT indicates that
the RTC offset is stored in UEFI on this particular machine.

Just as the nvmem-cell property indicates that the offset is stored in
some abstract nvmem, it seems reasonable to describe the offset being
stored in UEFI when that is the case (even if it is indeed generally
possible to probe for the latter).

An alternative might be to describe the efivars fw dependency in DT too
(e.g. for device links), but I believe you have already expressed some
concerns over that:

	https://lore.kernel.org/lkml/20230130210530.GA3339716-robh@kernel.org/

Johan
  
Ard Biesheuvel Feb. 9, 2023, 4:59 p.m. UTC | #3
On Wed, 1 Feb 2023 at 17:09, Johan Hovold <johan@kernel.org> wrote:
>
> [ +CC: Ard ]
>
> On Mon, Jan 30, 2023 at 12:49:44PM -0600, Rob Herring wrote:
> > On Thu, Jan 26, 2023 at 03:20:51PM +0100, Johan Hovold wrote:
> > > 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.
> > >
> > > Add a 'qcom,uefi-rtc-info' boolean flag which indicates that the RTC
> > > offset is stored in a Qualcomm specific UEFI variable so that the RTC
> > > time can be updated 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.
> >
> > Can't you just try to read the UEFI variable and use it if that
> > succeeds?
>
> Generally, yes. The problem here is that this UEFI variable is not used
> on all devices using these PMICs and I need a way to determine whether
> to wait for the UEFI variables to become available or not (e.g. when
> efivars support is built as module, yes, that's a thing now...).
>

Could we read this variable at boot and pass it to the kernel in a
different way? That way, we only have to defer the ability to set the
RTC, right?

> > I don't like this in DT because what if lots of devices start storing
> > lots of things in vendor specific UEFI variables. It doesn't scale.
>
> I hope we won't see that even if we already have some devices for x86
> platforms storing MAC addresses and such in UEFI variables. They
> currently access the UEFI firmware directly (i.e. not using the efivars
> abstraction) and simply assume UEFI is always there.
>
> With the Google SMI efivars implementation or the new Qualcomm SMC-based
> one, we need a way to determine whether to wait for efivars to become
> registered. For drivers where efivars is always needed we can just probe
> defer, but in this case we should not wait unless the DT indicates that
> the RTC offset is stored in UEFI on this particular machine.
>
> Just as the nvmem-cell property indicates that the offset is stored in
> some abstract nvmem, it seems reasonable to describe the offset being
> stored in UEFI when that is the case (even if it is indeed generally
> possible to probe for the latter).
>
> An alternative might be to describe the efivars fw dependency in DT too
> (e.g. for device links), but I believe you have already expressed some
> concerns over that:
>
>         https://lore.kernel.org/lkml/20230130210530.GA3339716-robh@kernel.org/
>
> Johan
  

Patch

diff --git a/Documentation/devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml b/Documentation/devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml
index b95a69cc9ae0..774c34c3f8f6 100644
--- a/Documentation/devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml
+++ b/Documentation/devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml
@@ -50,6 +50,12 @@  properties:
     items:
       - const: offset
 
+  qcom,uefi-rtc-info:
+    type: boolean
+    description:
+      RTC offset is stored as a four-byte GPS time offset in a 12-byte UEFI
+      variable 882f8c2b-9646-435f-8de5-f208ff80c1bd-RTCInfo
+
   wakeup-source: true
 
 required: