diff mbox series

arm64: dts: qcom: pmk8350: Make RTC disabled by default; enable on sc7280-idp

Message ID 20210929153553.1.Ib44c2ac967833d7a3f51452d44d15b7b8d23c1f0@changeid (mailing list archive)
State Accepted
Headers show
Series arm64: dts: qcom: pmk8350: Make RTC disabled by default; enable on sc7280-idp | expand

Commit Message

Doug Anderson Sept. 29, 2021, 10:38 p.m. UTC
The RTC on the pmk8350 is not useful on all boards. Some boards may
not provide backup power to the PMIC but might have another RTC on the
board that does have backup power. In this case it's better to not use
the RTC on the PMIC.

At the moment, the only boards that includes this PMIC are sc7280-idp
and sc7280-idp2. On sc7280-idp I'm not aware of any other RTCs, but
sc7280-idp2 has a Chrome OS EC on it and this is intended to provide
the RTC for the AP.

Let's do what we normally do for hardware that's not used by all
boards and set it to a default status of "disabled" and then enable it
on the boards that need it.

NOTE: for sc7280-idp it's _possible_ we might also want to add
`allow-set-time;`. That could be the subject of a future patch if it
is indeed true.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 arch/arm64/boot/dts/qcom/pmk8350.dtsi   | 1 +
 arch/arm64/boot/dts/qcom/sc7280-idp.dts | 4 ++++
 2 files changed, 5 insertions(+)

Comments

Matthias Kaehlcke Sept. 29, 2021, 11:36 p.m. UTC | #1
On Wed, Sep 29, 2021 at 03:38:14PM -0700, Douglas Anderson wrote:
> The RTC on the pmk8350 is not useful on all boards. Some boards may
> not provide backup power to the PMIC but might have another RTC on the
> board that does have backup power. In this case it's better to not use
> the RTC on the PMIC.
> 
> At the moment, the only boards that includes this PMIC are sc7280-idp
> and sc7280-idp2. On sc7280-idp I'm not aware of any other RTCs, but
> sc7280-idp2 has a Chrome OS EC on it and this is intended to provide
> the RTC for the AP.
> 
> Let's do what we normally do for hardware that's not used by all
> boards and set it to a default status of "disabled" and then enable it
> on the boards that need it.
> 
> NOTE: for sc7280-idp it's _possible_ we might also want to add
> `allow-set-time;`. That could be the subject of a future patch if it
> is indeed true.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Stephen Boyd Sept. 30, 2021, 12:03 a.m. UTC | #2
Quoting Douglas Anderson (2021-09-29 15:38:14)
> The RTC on the pmk8350 is not useful on all boards. Some boards may
> not provide backup power to the PMIC but might have another RTC on the
> board that does have backup power. In this case it's better to not use
> the RTC on the PMIC.
>
> At the moment, the only boards that includes this PMIC are sc7280-idp
> and sc7280-idp2. On sc7280-idp I'm not aware of any other RTCs, but
> sc7280-idp2 has a Chrome OS EC on it and this is intended to provide
> the RTC for the AP.
>
> Let's do what we normally do for hardware that's not used by all
> boards and set it to a default status of "disabled" and then enable it
> on the boards that need it.
>
> NOTE: for sc7280-idp it's _possible_ we might also want to add
> `allow-set-time;`. That could be the subject of a future patch if it
> is indeed true.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Satya Priya Sept. 30, 2021, 4 a.m. UTC | #3
On 2021-09-30 04:08, Douglas Anderson wrote:
> The RTC on the pmk8350 is not useful on all boards. Some boards may
> not provide backup power to the PMIC but might have another RTC on the
> board that does have backup power. In this case it's better to not use
> the RTC on the PMIC.
> 
> At the moment, the only boards that includes this PMIC are sc7280-idp
> and sc7280-idp2. On sc7280-idp I'm not aware of any other RTCs, but
> sc7280-idp2 has a Chrome OS EC on it and this is intended to provide
> the RTC for the AP.
> 
> Let's do what we normally do for hardware that's not used by all
> boards and set it to a default status of "disabled" and then enable it
> on the boards that need it.
> 
> NOTE: for sc7280-idp it's _possible_ we might also want to add
> `allow-set-time;`. That could be the subject of a future patch if it
> is indeed true.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> 

Reviewed-by: Satya Priya <skakit@codeaurora.org>
Doug Anderson Oct. 18, 2021, 9:45 p.m. UTC | #4
Bjorn,

On Wed, Sep 29, 2021 at 9:00 PM <skakit@codeaurora.org> wrote:
>
> On 2021-09-30 04:08, Douglas Anderson wrote:
> > The RTC on the pmk8350 is not useful on all boards. Some boards may
> > not provide backup power to the PMIC but might have another RTC on the
> > board that does have backup power. In this case it's better to not use
> > the RTC on the PMIC.
> >
> > At the moment, the only boards that includes this PMIC are sc7280-idp
> > and sc7280-idp2. On sc7280-idp I'm not aware of any other RTCs, but
> > sc7280-idp2 has a Chrome OS EC on it and this is intended to provide
> > the RTC for the AP.
> >
> > Let's do what we normally do for hardware that's not used by all
> > boards and set it to a default status of "disabled" and then enable it
> > on the boards that need it.
> >
> > NOTE: for sc7280-idp it's _possible_ we might also want to add
> > `allow-set-time;`. That could be the subject of a future patch if it
> > is indeed true.
> >
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > ---
> >
>
> Reviewed-by: Satya Priya <skakit@codeaurora.org>

If you're still accepting patches for 5.16, it'd be keen if you'd
consider taking this one. Thanks!

-Doug
Bjorn Andersson Oct. 24, 2021, 3:54 a.m. UTC | #5
On Mon 18 Oct 16:45 CDT 2021, Doug Anderson wrote:

> Bjorn,
> 
> On Wed, Sep 29, 2021 at 9:00 PM <skakit@codeaurora.org> wrote:
> >
> > On 2021-09-30 04:08, Douglas Anderson wrote:
> > > The RTC on the pmk8350 is not useful on all boards. Some boards may
> > > not provide backup power to the PMIC but might have another RTC on the
> > > board that does have backup power. In this case it's better to not use
> > > the RTC on the PMIC.
> > >
> > > At the moment, the only boards that includes this PMIC are sc7280-idp
> > > and sc7280-idp2. On sc7280-idp I'm not aware of any other RTCs, but
> > > sc7280-idp2 has a Chrome OS EC on it and this is intended to provide
> > > the RTC for the AP.
> > >
> > > Let's do what we normally do for hardware that's not used by all
> > > boards and set it to a default status of "disabled" and then enable it
> > > on the boards that need it.
> > >
> > > NOTE: for sc7280-idp it's _possible_ we might also want to add
> > > `allow-set-time;`. That could be the subject of a future patch if it
> > > is indeed true.
> > >
> > > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > > ---
> > >
> >
> > Reviewed-by: Satya Priya <skakit@codeaurora.org>
> 
> If you're still accepting patches for 5.16, it'd be keen if you'd
> consider taking this one. Thanks!
> 

I've picked the patch and hope to get it included in v5.16.

I do however not understand why the commit message so clearly defines
that the only device including pmk8350 is the sc7280 idp when the
sm8350-mtp.dts contains the following line:

#include "pmk8350.dtsi"


Perhaps I'm missing something obvious, but I took the liberty of also
enabling the RTC in the SM8350 MTP.

Regards,
Bjorn
Doug Anderson Oct. 27, 2021, 11:13 p.m. UTC | #6
Hi,

On Sat, Oct 23, 2021 at 8:54 PM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> On Mon 18 Oct 16:45 CDT 2021, Doug Anderson wrote:
>
> > Bjorn,
> >
> > On Wed, Sep 29, 2021 at 9:00 PM <skakit@codeaurora.org> wrote:
> > >
> > > On 2021-09-30 04:08, Douglas Anderson wrote:
> > > > The RTC on the pmk8350 is not useful on all boards. Some boards may
> > > > not provide backup power to the PMIC but might have another RTC on the
> > > > board that does have backup power. In this case it's better to not use
> > > > the RTC on the PMIC.
> > > >
> > > > At the moment, the only boards that includes this PMIC are sc7280-idp
> > > > and sc7280-idp2. On sc7280-idp I'm not aware of any other RTCs, but
> > > > sc7280-idp2 has a Chrome OS EC on it and this is intended to provide
> > > > the RTC for the AP.
> > > >
> > > > Let's do what we normally do for hardware that's not used by all
> > > > boards and set it to a default status of "disabled" and then enable it
> > > > on the boards that need it.
> > > >
> > > > NOTE: for sc7280-idp it's _possible_ we might also want to add
> > > > `allow-set-time;`. That could be the subject of a future patch if it
> > > > is indeed true.
> > > >
> > > > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > > > ---
> > > >
> > >
> > > Reviewed-by: Satya Priya <skakit@codeaurora.org>
> >
> > If you're still accepting patches for 5.16, it'd be keen if you'd
> > consider taking this one. Thanks!
> >
>
> I've picked the patch and hope to get it included in v5.16.
>
> I do however not understand why the commit message so clearly defines
> that the only device including pmk8350 is the sc7280 idp when the
> sm8350-mtp.dts contains the following line:
>
> #include "pmk8350.dtsi"
>
>
> Perhaps I'm missing something obvious, but I took the liberty of also
> enabling the RTC in the SM8350 MTP.

Thank you for fixing this up. I must have just glazed over that part
of things. :( Glad you caught and fixed it.

-Doug
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/pmk8350.dtsi b/arch/arm64/boot/dts/qcom/pmk8350.dtsi
index 04fc2632a0b2..769f9726806f 100644
--- a/arch/arm64/boot/dts/qcom/pmk8350.dtsi
+++ b/arch/arm64/boot/dts/qcom/pmk8350.dtsi
@@ -59,6 +59,7 @@  pmk8350_rtc: rtc@6100 {
 			reg = <0x6100>, <0x6200>;
 			reg-names = "rtc", "alarm";
 			interrupts = <0x0 0x62 0x1 IRQ_TYPE_EDGE_RISING>;
+			status = "disabled";
 		};
 
 		pmk8350_gpios: gpio@b000 {
diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
index 64fc22aff33d..e11412bae738 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
@@ -61,6 +61,10 @@  &ipa {
 	modem-init;
 };
 
+&pmk8350_rtc {
+	status = "okay";
+};
+
 &pmk8350_vadc {
 	pmr735a_die_temp {
 		reg = <PMR735A_ADC7_DIE_TEMP>;