2019-06-20 22:59:59

by Luca Weiss

[permalink] [raw]
Subject: [PATCH] ARM: dts: msm8974-FP2: add reboot-mode node

This enables userspace to signal the bootloader to go into the
bootloader or recovery mode.

The magic values can be found in both the downstream kernel and the LK
kernel (bootloader).

Signed-off-by: Luca Weiss <[email protected]>
---
Sidenote: Why are there no userspace tools to be found that support
this? Anyways, we have one now in postmarketOS :)

arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts | 14 ++++++++++++++
1 file changed, 14 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts b/arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts
index 643c57f84818..f86736a6d77e 100644
--- a/arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts
+++ b/arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts
@@ -338,6 +338,20 @@
};
};
};
+
+ imem@fe805000 {
+ compatible = "syscon", "simple-mfd";
+ reg = <0xfe805000 0x1000>;
+
+ reboot-mode {
+ compatible = "syscon-reboot-mode";
+ offset = <0x65c>;
+
+ mode-normal = <0x77665501>;
+ mode-bootloader = <0x77665500>;
+ mode-recovery = <0x77665502>;
+ };
+ };
};

&spmi_bus {
--
2.22.0


2019-06-21 00:01:53

by Brian Masney

[permalink] [raw]
Subject: Re: [PATCH] ARM: dts: msm8974-FP2: add reboot-mode node

Hi Luca,

On Fri, Jun 21, 2019 at 12:58:24AM +0200, Luca Weiss wrote:
> This enables userspace to signal the bootloader to go into the
> bootloader or recovery mode.
>
> The magic values can be found in both the downstream kernel and the LK
> kernel (bootloader).
>
> Signed-off-by: Luca Weiss <[email protected]>
> ---
> Sidenote: Why are there no userspace tools to be found that support
> this? Anyways, we have one now in postmarketOS :)
>
> arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts b/arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts
> index 643c57f84818..f86736a6d77e 100644
> --- a/arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts
> +++ b/arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts
> @@ -338,6 +338,20 @@
> };
> };
> };
> +
> + imem@fe805000 {
> + compatible = "syscon", "simple-mfd";
> + reg = <0xfe805000 0x1000>;
> +
> + reboot-mode {
> + compatible = "syscon-reboot-mode";
> + offset = <0x65c>;
> +
> + mode-normal = <0x77665501>;
> + mode-bootloader = <0x77665500>;
> + mode-recovery = <0x77665502>;
> + };
> + };
> };

I think that it makes sense to put this snippet in qcom-msm8974.dtsi
with a status of disabled, and then enable it in
qcom-msm8974-fairphone-fp2.dts like so:

imem@fe805000 {
status = "ok";
};

What's the pmOS utility that utilizes this? I'll test it on the Nexus 5.

Thanks,

Brian

2019-06-21 19:25:47

by Luca Weiss

[permalink] [raw]
Subject: Re: [PATCH] ARM: dts: msm8974-FP2: add reboot-mode node

On Freitag, 21. Juni 2019 02:01:22 CEST you wrote:
> I think that it makes sense to put this snippet in qcom-msm8974.dtsi
> with a status of disabled, and then enable it in
> qcom-msm8974-fairphone-fp2.dts like so:
>
> imem@fe805000 {
> status = "ok";
> };

Do you want me to put the whole node in the the dtsi file? Even though these
values are the same, there are also custom vendor-specified values for specific
phones.

This opens another question, which values we should put into the dts files. For
example in the fairphone 2 bootloader source there's also the unused
#define ALARM_BOOT 0x77665503

and behind a #if VERIFIED_BOOT :
#define DM_VERITY_LOGGING 0x77665508
#define DM_VERITY_ENFORCING 0x77665509
#define DM_VERITY_KEYSCLEAR 0x7766550A

and 0x77665501 ("mode-normal") isn't used in the bootloader at all.

On the Linux kernel side, it has bootloader (0x77665500), recovery
(0x77665502), rtc (0x77665503), oem-* (0x6f656d00 | somevalue), edl (some
other addresses), and the else statements writes the 0x77665501 value in my
patch.

> What's the pmOS utility that utilizes this? I'll test it on the Nexus 5.

"reboot-mode" at https://gitlab.com/postmarketOS/pmaports/merge_requests/442

> Thanks,
>
> Brian


Attachments:
signature.asc (849.00 B)
This is a digitally signed message part.

2019-06-22 01:43:35

by Brian Masney

[permalink] [raw]
Subject: Re: [PATCH] ARM: dts: msm8974-FP2: add reboot-mode node

On Fri, Jun 21, 2019 at 09:25:17PM +0200, Luca Weiss wrote:
> On Freitag, 21. Juni 2019 02:01:22 CEST you wrote:
> > I think that it makes sense to put this snippet in qcom-msm8974.dtsi
> > with a status of disabled, and then enable it in
> > qcom-msm8974-fairphone-fp2.dts like so:
> >
> > imem@fe805000 {
> > status = "ok";
> > };
>
> Do you want me to put the whole node in the the dtsi file? Even though these
> values are the same, there are also custom vendor-specified values for specific
> phones.

mach-msm in the downstream hammerhead sources has those addresses:
https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/arch/arm/mach-msm/restart.c#L271
This lead me to think that it applies to other msm8974-based systems as
well.

I tried your device tree snippet on the Nexus 5 and it reboots the phone
for me.

/ # ./reboot-mode normal
[ 85.088556] reboot: Restarting system with command 'normal'

The recovery and bootloader modes reboot the phone but into normal mode.
Oddly, the bootloader shows different power on reasons after the
"welcome to hammerhead bootloader" message.

normal = [10] Power on reason 20001
recovery = [10] Power on reason 1
bootloader = [10] Power on reason 20001

> On the Linux kernel side, it has bootloader (0x77665500), recovery
> (0x77665502), rtc (0x77665503), oem-* (0x6f656d00 | somevalue), edl (some
> other addresses), and the else statements writes the 0x77665501 value in my
> patch.

The downstream hammerhead sources have the oem-*, and emergency download
modes (edl) listed as well.

I'm not sure on your other questions.

Brian

2019-07-13 11:28:09

by Luca Weiss

[permalink] [raw]
Subject: Re: [PATCH] ARM: dts: msm8974-FP2: add reboot-mode node

Hi Brian,
how about something like that (formatting is surely broken because I'm not
sending this with git-send-email^^)?

I'd says this should be work fine with all devices as all modes are defined in
the device-specific dts but the reg and offset values are in the board dts.
Should I also add a status = "disabled" to the reboot-mode node in the board
dts?

diff --git a/arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts b/arch/arm/boot/
dts/qcom-msm8974-fairphone-fp2.dts
index 643c57f84818..ff4a3e0aa746 100644
--- a/arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts
+++ b/arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts
@@ -338,6 +338,16 @@
};
};
};
+
+ imem@fe805000 {
+ status = "okay";
+
+ reboot-mode {
+ mode-normal = <0x77665501>;
+ mode-bootloader = <0x77665500>;
+ mode-recovery = <0x77665502>;
+ };
+ };
};

&spmi_bus {
diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-
msm8974.dtsi
index 45b5c8ef0374..1927430bded7 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -1085,6 +1085,17 @@
};
};
};
+
+ imem@fe805000 {
+ status = "disabled";
+ compatible = "syscon", "simple-mfd";
+ reg = <0xfe805000 0x1000>;
+
+ reboot-mode {
+ compatible = "syscon-reboot-mode";
+ offset = <0x65c>;
+ };
+ };
};

smd {


Regards,
Luca


2019-07-13 14:31:43

by Brian Masney

[permalink] [raw]
Subject: Re: [PATCH] ARM: dts: msm8974-FP2: add reboot-mode node

On Sat, Jul 13, 2019 at 01:26:45PM +0200, Luca Weiss wrote:
> Hi Brian,
> how about something like that (formatting is surely broken because I'm not
> sending this with git-send-email^^)?
>
> I'd says this should be work fine with all devices as all modes are defined in
> the device-specific dts but the reg and offset values are in the board dts.
> Should I also add a status = "disabled" to the reboot-mode node in the board
> dts?
>
> diff --git a/arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts b/arch/arm/boot/
> dts/qcom-msm8974-fairphone-fp2.dts
> index 643c57f84818..ff4a3e0aa746 100644
> --- a/arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts
> +++ b/arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts
> @@ -338,6 +338,16 @@
> };
> };
> };
> +
> + imem@fe805000 {
> + status = "okay";
> +
> + reboot-mode {
> + mode-normal = <0x77665501>;
> + mode-bootloader = <0x77665500>;
> + mode-recovery = <0x77665502>;
> + };
> + };
> };
>
> &spmi_bus {
> diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-
> msm8974.dtsi
> index 45b5c8ef0374..1927430bded7 100644
> --- a/arch/arm/boot/dts/qcom-msm8974.dtsi
> +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
> @@ -1085,6 +1085,17 @@
> };
> };
> };
> +
> + imem@fe805000 {
> + status = "disabled";
> + compatible = "syscon", "simple-mfd";
> + reg = <0xfe805000 0x1000>;
> +
> + reboot-mode {
> + compatible = "syscon-reboot-mode";
> + offset = <0x65c>;
> + };
> + };
> };
>
> smd {

I think this sounds reasonable.

Reviewed-by: Brian Masney <[email protected]>

You should resend this out with git send-email and see what Bjorn says.

Brian