[PATCH v3 0/5] x1e80100 RTC support

[PATCH v3 0/5] x1e80100 RTC support

[Jonathan Marek]

x1e80100 needs a workaround because the RTC alarm is not owned by HLOS.
It also needs the same offset workaround as sc8280xp/etc.

v2: remove duplicated ops and use RTC_FEATURE_ALARM instead
v3:
 - renamed flag to qcom,no-alarm
 - don't remove alarm registers/interrupt from dts

Jonathan Marek (5):
  rtc: pm8xxx: implement qcom,no-alarm flag for non-HLOS owned alarm
  dt-bindings: rtc: qcom-pm8xxx: document qcom,no-alarm flag
  arm64: dts: qcom: x1e80100-pmics: enable RTC
  arm64: dts: qcom: x1e80100-crd: add rtc offset to set rtc time
  arm64: dts: qcom: x1e78100-t14s: add rtc offset to set rtc time

 .../bindings/rtc/qcom-pm8xxx-rtc.yaml         |  5 +++
 .../qcom/x1e78100-lenovo-thinkpad-t14s.dts    | 11 +++++
 arch/arm64/boot/dts/qcom/x1e80100-crd.dts     | 11 +++++
 arch/arm64/boot/dts/qcom/x1e80100-pmics.dtsi  |  3 +-
 drivers/rtc/rtc-pm8xxx.c                      | 44 +++++++++++++------
 5 files changed, 58 insertions(+), 16 deletions(-)

-- 
2.45.1

[PATCH v3 2/5] dt-bindings: rtc: qcom-pm8xxx: document qcom,no-alarm flag

[Jonathan Marek]

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 <jonathan@marek.ca>
---
 Documentation/devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml b/Documentation/devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml
index d274bb7a534b5..23a5316efadba 100644
--- a/Documentation/devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml
+++ b/Documentation/devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml
@@ -40,6 +40,11 @@ properties:
     description:
       Indicates that the setting of RTC time is allowed by the host CPU.
 
+  qcom,no-alarm:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      Indicates that RTC alarm is not owned by HLOS (Linux).
+
   nvmem-cells:
     items:
       - description:
-- 
2.45.1

Re: [PATCH v3 2/5] dt-bindings: rtc: qcom-pm8xxx: document qcom,no-alarm flag

[Krzysztof Kozlowski]

On 15/10/2024 02:47, Jonathan Marek wrote:
> 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 <jonathan@marek.ca>
> ---

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v3 2/5] dt-bindings: rtc: qcom-pm8xxx: document qcom,no-alarm flag

[Johan Hovold]

On Mon, Oct 14, 2024 at 08:47:27PM -0400, Jonathan Marek wrote:
> 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.

Could dropping the alarm interrupt be an alternative to another vendor
property? Some existing RTC drivers use this.

It also seems like having a dedicated compatible for this platform is
warranted, and would also suffice to determine when the alarm registers
are writable.

Note that the binding update should go before the driver patch in the
series.

Johan

[PATCH v3 3/5] arm64: dts: qcom: x1e80100-pmics: enable RTC

[Jonathan Marek]

Only access to RTC alarm is blocked.

Enable RTC access by setting the qcom,no-alarm flag.

Signed-off-by: Jonathan Marek <jonathan@marek.ca>
---
 arch/arm64/boot/dts/qcom/x1e80100-pmics.dtsi | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/x1e80100-pmics.dtsi b/arch/arm64/boot/dts/qcom/x1e80100-pmics.dtsi
index 5b54ee79f048e..b665cb75c2b75 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100-pmics.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1e80100-pmics.dtsi
@@ -223,8 +223,7 @@ pmk8550_rtc: rtc@6100 {
 			reg = <0x6100>, <0x6200>;
 			reg-names = "rtc", "alarm";
 			interrupts = <0x0 0x62 0x1 IRQ_TYPE_EDGE_RISING>;
-			/* Not yet sure what blocks access */
-			status = "reserved";
+			qcom,no-alarm; /* alarm owned by ADSP */
 		};
 
 		pmk8550_sdam_2: nvram@7100 {
-- 
2.45.1

[PATCH v3 4/5] arm64: dts: qcom: x1e80100-crd: add rtc offset to set rtc time

[Jonathan Marek]

See commit e67b45582c5e for explanation.

Note: the 0xbc offset is arbitrary, it just needs to not be already in use.

Signed-off-by: Jonathan Marek <jonathan@marek.ca>
---
 arch/arm64/boot/dts/qcom/x1e80100-crd.dts | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/x1e80100-crd.dts b/arch/arm64/boot/dts/qcom/x1e80100-crd.dts
index 6dfc85eda3540..eb6b735c41453 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100-crd.dts
+++ b/arch/arm64/boot/dts/qcom/x1e80100-crd.dts
@@ -1224,6 +1224,17 @@ edp_bl_en: edp-bl-en-state {
 	};
 };
 
+&pmk8550_rtc {
+	nvmem-cells = <&rtc_offset>;
+	nvmem-cell-names = "offset";
+};
+
+&pmk8550_sdam_2 {
+	rtc_offset: rtc-offset@bc {
+		reg = <0xbc 0x4>;
+	};
+};
+
 &qupv3_0 {
 	status = "okay";
 };
-- 
2.45.1

Re: [PATCH v3 4/5] arm64: dts: qcom: x1e80100-crd: add rtc offset to set rtc time

[Johan Hovold]

On Mon, Oct 14, 2024 at 08:47:29PM -0400, Jonathan Marek wrote:
> See commit e67b45582c5e for explanation.

It's good that you reference commit e67b45582c5e ("arm64: dts: qcom:
sc8280xp-crd: enable rtc") but your commit message still needs to be
self-contained and provide the explanation here in some form (e.g.
quoted or paraphrased).

Also spell out the commit summary in parenthesis when referring to
commits as I did above.

> Note: the 0xbc offset is arbitrary, it just needs to not be already in use.

How did you verify that nothing is using this offset on this platform? I
assume we need someone with access to the docs to make sure it's not in
use as we did for sc8280xp.

Johan

Re: [PATCH v3 4/5] arm64: dts: qcom: x1e80100-crd: add rtc offset to set rtc time

[Jonathan Marek]

On 10/16/24 2:51 AM, Johan Hovold wrote:
> On Mon, Oct 14, 2024 at 08:47:29PM -0400, Jonathan Marek wrote:
>> See commit e67b45582c5e for explanation.
> 
> It's good that you reference commit e67b45582c5e ("arm64: dts: qcom:
> sc8280xp-crd: enable rtc") but your commit message still needs to be
> self-contained and provide the explanation here in some form (e.g.
> quoted or paraphrased).
> 
> Also spell out the commit summary in parenthesis when referring to
> commits as I did above.
> 
>> Note: the 0xbc offset is arbitrary, it just needs to not be already in use.
> 
> How did you verify that nothing is using this offset on this platform? I
> assume we need someone with access to the docs to make sure it's not in
> use as we did for sc8280xp.
> 

AFAIK qcom allocate things from the start of the SDAM, so allocating 
from the end of the SDAM should be safe. And AFAIK this is supposed to 
be a general purpose HLOS (linux/windows) SDAM block, so should be 
mostly free to use.

(its possible windows uses this offset for something, I don't know about 
that)

Re: [PATCH v3 4/5] arm64: dts: qcom: x1e80100-crd: add rtc offset to set rtc time

[Johan Hovold]

On Wed, Oct 16, 2024 at 09:31:00AM -0400, Jonathan Marek wrote:
> On 10/16/24 2:51 AM, Johan Hovold wrote:
> > On Mon, Oct 14, 2024 at 08:47:29PM -0400, Jonathan Marek wrote:

> >> Note: the 0xbc offset is arbitrary, it just needs to not be already in use.
> > 
> > How did you verify that nothing is using this offset on this platform? I
> > assume we need someone with access to the docs to make sure it's not in
> > use as we did for sc8280xp.
>
> AFAIK qcom allocate things from the start of the SDAM, so allocating 
> from the end of the SDAM should be safe. And AFAIK this is supposed to 
> be a general purpose HLOS (linux/windows) SDAM block, so should be 
> mostly free to use.

From what I understand these registers are also used for things like
programmable LEDs (e.g. see 24e2d05d1b68 ("leds: Add driver for Qualcomm
LPG")). And who knows what else.

It would be good if someone from Qualcomm could confirm that these bytes
are free for use before merging. I've started asking around.

Johan

Re: [PATCH v3 4/5] arm64: dts: qcom: x1e80100-crd: add rtc offset to set rtc time

[Konrad Dybcio]

On 15.10.2024 2:47 AM, Jonathan Marek wrote:
> See commit e67b45582c5e for explanation.
> 
> Note: the 0xbc offset is arbitrary, it just needs to not be already in use.
> 
> Signed-off-by: Jonathan Marek <jonathan@marek.ca>
> ---
>  arch/arm64/boot/dts/qcom/x1e80100-crd.dts | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/x1e80100-crd.dts b/arch/arm64/boot/dts/qcom/x1e80100-crd.dts
> index 6dfc85eda3540..eb6b735c41453 100644
> --- a/arch/arm64/boot/dts/qcom/x1e80100-crd.dts
> +++ b/arch/arm64/boot/dts/qcom/x1e80100-crd.dts
> @@ -1224,6 +1224,17 @@ edp_bl_en: edp-bl-en-state {
>  	};
>  };
>  
> +&pmk8550_rtc {
> +	nvmem-cells = <&rtc_offset>;
> +	nvmem-cell-names = "offset";
> +};
> +
> +&pmk8550_sdam_2 {
> +	rtc_offset: rtc-offset@bc {
> +		reg = <0xbc 0x4>;
> +	};
> +};

Setting random bits in SDAM is a very very very very bad idea

I'll try to get a good spot for the offset internally

Konrad

[PATCH v3 5/5] arm64: dts: qcom: x1e78100-t14s: add rtc offset to set rtc time

[Jonathan Marek]

See commit e67b45582c5e for explanation.

Note: the 0xbc offset is arbitrary, it just needs to not be already in use.

Signed-off-by: Jonathan Marek <jonathan@marek.ca>
---
 .../boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts   | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts
index d18461c545547..f05523cb51cd4 100644
--- a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts
+++ b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts
@@ -897,6 +897,17 @@ edp_bl_en: edp-bl-en-state {
 	};
 };
 
+&pmk8550_rtc {
+	nvmem-cells = <&rtc_offset>;
+	nvmem-cell-names = "offset";
+};
+
+&pmk8550_sdam_2 {
+	rtc_offset: rtc-offset@bc {
+		reg = <0xbc 0x4>;
+	};
+};
+
 &qupv3_0 {
 	status = "okay";
 };
-- 
2.45.1

Re: [PATCH v3 0/5] x1e80100 RTC support

[Alexandre Belloni]

Hello,

Did I miss v4 of this series?

On 14/10/2024 20:47:25-0400, Jonathan Marek wrote:
> x1e80100 needs a workaround because the RTC alarm is not owned by HLOS.
> It also needs the same offset workaround as sc8280xp/etc.
> 
> v2: remove duplicated ops and use RTC_FEATURE_ALARM instead
> v3:
>  - renamed flag to qcom,no-alarm
>  - don't remove alarm registers/interrupt from dts
> 
> Jonathan Marek (5):
>   rtc: pm8xxx: implement qcom,no-alarm flag for non-HLOS owned alarm
>   dt-bindings: rtc: qcom-pm8xxx: document qcom,no-alarm flag
>   arm64: dts: qcom: x1e80100-pmics: enable RTC
>   arm64: dts: qcom: x1e80100-crd: add rtc offset to set rtc time
>   arm64: dts: qcom: x1e78100-t14s: add rtc offset to set rtc time
> 
>  .../bindings/rtc/qcom-pm8xxx-rtc.yaml         |  5 +++
>  .../qcom/x1e78100-lenovo-thinkpad-t14s.dts    | 11 +++++
>  arch/arm64/boot/dts/qcom/x1e80100-crd.dts     | 11 +++++
>  arch/arm64/boot/dts/qcom/x1e80100-pmics.dtsi  |  3 +-
>  drivers/rtc/rtc-pm8xxx.c                      | 44 +++++++++++++------
>  5 files changed, 58 insertions(+), 16 deletions(-)
> 
> -- 
> 2.45.1
> 
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v3 0/5] x1e80100 RTC support

[Johan Hovold]

Hi Alexandre and Jonathan,

On Mon, Jan 13, 2025 at 12:35:38AM +0100, Alexandre Belloni wrote:
> Did I miss v4 of this series?

We are still waiting for Qualcomm to provide a free and safe-to-use
allocation for the RTC offset in the PMIC.

I got tired of reminding the Qualcomm folks about this and instead
revived my patches for using the UEFI variable that the firmware (and
Windows) use for the offset.

I included Jonathan's v3 series after making the changes that we
discussed here (and a few more).

The result is here:

	https://lore.kernel.org/lkml/20250120144152.11949-1-johan+linaro@kernel.org/

and should allow us to enable the RTC on X1E today (and also avoids
running into the same SDAM allocation issue for the next platform).

> On 14/10/2024 20:47:25-0400, Jonathan Marek wrote:
> > x1e80100 needs a workaround because the RTC alarm is not owned by HLOS.
> > It also needs the same offset workaround as sc8280xp/etc.
> > 
> > v2: remove duplicated ops and use RTC_FEATURE_ALARM instead
> > v3:
> >  - renamed flag to qcom,no-alarm
> >  - don't remove alarm registers/interrupt from dts
> > 
> > Jonathan Marek (5):
> >   rtc: pm8xxx: implement qcom,no-alarm flag for non-HLOS owned alarm
> >   dt-bindings: rtc: qcom-pm8xxx: document qcom,no-alarm flag
> >   arm64: dts: qcom: x1e80100-pmics: enable RTC
> >   arm64: dts: qcom: x1e80100-crd: add rtc offset to set rtc time
> >   arm64: dts: qcom: x1e78100-t14s: add rtc offset to set rtc time

Johan