2023-04-13 13:21:09

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH 0/4] arm64: qcom: fix the reboot reason handling on sa8775p

From: Bartosz Golaszewski <[email protected]>

SA8775P uses nvmem to pass the reboot reason magic value to the bootloader.
Remove the reboot modes from the PON node and introduce an SDAM node passed
to the nvmem-reboot-mode driver. While at it: convert the bindings for
nvmem-reboot-mode to YAML and enable it for arm64 in defconfig.

Bartosz Golaszewski (2):
arm64: defconfig: enable building the nvmem-reboot-mode module
dt-bindings: power: reset: convert nvmem-reboot-mode bindings to YAML

Parikshit Pareek (2):
arm64: dts: qcom: sa8775p: pmic: remove the PON modes
arm64: dts: qcom: sa8775p: pmic: add the sdam_0 node

.../power/reset/nvmem-reboot-mode.txt | 26 ----------
.../power/reset/nvmem-reboot-mode.yaml | 52 +++++++++++++++++++
arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi | 23 +++++++-
arch/arm64/configs/defconfig | 1 +
4 files changed, 74 insertions(+), 28 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/power/reset/nvmem-reboot-mode.txt
create mode 100644 Documentation/devicetree/bindings/power/reset/nvmem-reboot-mode.yaml

--
2.37.2


2023-04-13 13:22:07

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH 1/4] arm64: dts: qcom: sa8775p: pmic: remove the PON modes

From: Parikshit Pareek <[email protected]>

Remove the power on reasons with reboot from the pmm8654au_0_pon.
Instead, the PoN reaons should be part of different sdam_0 mode, to
be interoduced.

Signed-off-by: Parikshit Pareek <[email protected]>
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi | 2 --
1 file changed, 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi
index 7602cca47bae..5abdc239d3a6 100644
--- a/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi
@@ -108,8 +108,6 @@ pmm8654au_0_pon: pon@1200 {
compatible = "qcom,pmk8350-pon";
reg = <0x1200>, <0x800>;
reg-names = "hlos", "pbs";
- mode-recovery = <0x1>;
- mode-bootloader = <0x2>;

pmm8654au_0_pon_pwrkey: pwrkey {
compatible = "qcom,pmk8350-pwrkey";
--
2.37.2

2023-04-13 13:23:50

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH 2/4] arm64: defconfig: enable building the nvmem-reboot-mode module

From: Bartosz Golaszewski <[email protected]>

This module is used by the Qualcomm sa8775p platform for passing the
reboot reason to the bootloader. Enable building it in the arm64
defconfig as a module.

Signed-off-by: Bartosz Golaszewski <[email protected]>
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index e1063ab32658..5bdc9cede807 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -614,6 +614,7 @@ CONFIG_POWER_RESET_QCOM_PON=m
CONFIG_POWER_RESET_XGENE=y
CONFIG_POWER_RESET_SYSCON=y
CONFIG_SYSCON_REBOOT_MODE=y
+CONFIG_NVMEM_REBOOT_MODE=m
CONFIG_BATTERY_SBS=m
CONFIG_BATTERY_BQ27XXX=y
CONFIG_BATTERY_MAX17042=m
--
2.37.2

2023-04-13 13:24:16

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH 3/4] dt-bindings: power: reset: convert nvmem-reboot-mode bindings to YAML

From: Bartosz Golaszewski <[email protected]>

Convert the DT binding document for nvmem-reboot-mode from .txt to YAML.

Signed-off-by: Bartosz Golaszewski <[email protected]>
---
.../power/reset/nvmem-reboot-mode.txt | 26 ----------
.../power/reset/nvmem-reboot-mode.yaml | 52 +++++++++++++++++++
2 files changed, 52 insertions(+), 26 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/power/reset/nvmem-reboot-mode.txt
create mode 100644 Documentation/devicetree/bindings/power/reset/nvmem-reboot-mode.yaml

diff --git a/Documentation/devicetree/bindings/power/reset/nvmem-reboot-mode.txt b/Documentation/devicetree/bindings/power/reset/nvmem-reboot-mode.txt
deleted file mode 100644
index 752d6126d5da..000000000000
--- a/Documentation/devicetree/bindings/power/reset/nvmem-reboot-mode.txt
+++ /dev/null
@@ -1,26 +0,0 @@
-NVMEM reboot mode driver
-
-This driver gets reboot mode magic value from reboot-mode driver
-and stores it in a NVMEM cell named "reboot-mode". Then the bootloader
-can read it and take different action according to the magic
-value stored.
-
-Required properties:
-- compatible: should be "nvmem-reboot-mode".
-- nvmem-cells: A phandle to the reboot mode provided by a nvmem device.
-- nvmem-cell-names: Should be "reboot-mode".
-
-The rest of the properties should follow the generic reboot-mode description
-found in reboot-mode.txt
-
-Example:
- reboot-mode {
- compatible = "nvmem-reboot-mode";
- nvmem-cells = <&reboot_mode>;
- nvmem-cell-names = "reboot-mode";
-
- mode-normal = <0xAAAA5501>;
- mode-bootloader = <0xBBBB5500>;
- mode-recovery = <0xCCCC5502>;
- mode-test = <0xDDDD5503>;
- };
diff --git a/Documentation/devicetree/bindings/power/reset/nvmem-reboot-mode.yaml b/Documentation/devicetree/bindings/power/reset/nvmem-reboot-mode.yaml
new file mode 100644
index 000000000000..64a7d224c7dd
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/reset/nvmem-reboot-mode.yaml
@@ -0,0 +1,52 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/reset/nvmem-reboot-mode.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Generic NVMEM reboot mode driver
+
+maintainers:
+ - Bartosz Golaszewski <[email protected]>
+
+description: |
+ This driver gets the reboot mode magic value from the reboot-mode driver
+ and stores it in the NVMEM cell named "reboot-mode". The bootloader can
+ then read it and take different action according to the value.
+
+properties:
+ compatible:
+ const: nvmem-reboot-mode
+
+ nvmem-cells:
+ description: |
+ A phandle pointing to the nvmem-cells node where the vendor-specific
+ magic value representing the reboot mode is stored.
+ maxItems: 1
+
+ nvmem-cell-names:
+ items:
+ - const: reboot-mode
+
+patternProperties:
+ "^mode-.+":
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: Vendor-specific mode value written to the mode register
+
+additionalProperties: false
+
+required:
+ - compatible
+ - nvmem-cells
+ - nvmem-cell-names
+
+examples:
+ - |
+ reboot-mode {
+ compatible = "nvmem-reboot-mode";
+ nvmem-cells = <&reboot_reason>;
+ nvmem-cell-names = "reboot-mode";
+ mode-recovery = <0x01>;
+ mode-bootloader = <0x02>;
+ };
+...
--
2.37.2

2023-04-13 13:25:10

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH 4/4] arm64: dts: qcom: sa8775p: pmic: add the sdam_0 node

From: Parikshit Pareek <[email protected]>

Introduce sdam_0 node, which is to be used via nvmem for power on
reasons during reboot. Add supported PoN reaons supported via sdam_0
node.

Signed-off-by: Parikshit Pareek <[email protected]>
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi
index 5abdc239d3a6..49bf7b08f5b6 100644
--- a/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi
@@ -88,6 +88,14 @@ trip1 {
};
};
};
+
+ reboot_reason {
+ compatible = "nvmem-reboot-mode";
+ nvmem-cells = <&reboot_reason>;
+ nvmem-cell-names = "reboot-mode";
+ mode-recovery = <0x01>;
+ mode-bootloader = <0x02>;
+ };
};

&spmi_bus {
@@ -133,6 +141,19 @@ pmm8654au_0_gpios: gpio@8800 {
interrupt-controller;
#interrupt-cells = <2>;
};
+
+ pmm8654au_0_sdam_0: nvram@7100 {
+ compatible = "qcom,spmi-sdam";
+ reg = <0x7100>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x7100 0x100>;
+
+ reboot_reason: reboot-reason@48 {
+ reg = <0x48 0x1>;
+ bits = <1 7>;
+ };
+ };
};

pmm8654au_1: pmic@2 {
--
2.37.2

2023-04-13 16:14:32

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/4] arm64: dts: qcom: sa8775p: pmic: remove the PON modes

On 13/04/2023 15:17, Bartosz Golaszewski wrote:
> From: Parikshit Pareek <[email protected]>
>
> Remove the power on reasons with reboot from the pmm8654au_0_pon.
> Instead, the PoN reaons should be part of different sdam_0 mode, to

typo: reasons

> be interoduced.

introduced

Anyway it does not say why. Are these power reasons not correct?

>
> Signed-off-by: Parikshit Pareek <[email protected]>
> Signed-off-by: Bartosz Golaszewski <[email protected]>
> ---

Best regards,
Krzysztof

2023-04-13 16:32:23

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 4/4] arm64: dts: qcom: sa8775p: pmic: add the sdam_0 node

On 13/04/2023 15:17, Bartosz Golaszewski wrote:
> From: Parikshit Pareek <[email protected]>
>
> Introduce sdam_0 node, which is to be used via nvmem for power on
> reasons during reboot. Add supported PoN reaons supported via sdam_0
> node.
>
> Signed-off-by: Parikshit Pareek <[email protected]>
> Signed-off-by: Bartosz Golaszewski <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi
> index 5abdc239d3a6..49bf7b08f5b6 100644
> --- a/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi
> @@ -88,6 +88,14 @@ trip1 {
> };
> };
> };
> +
> + reboot_reason {

No underscores in node names. This is almost always called reboot-mode.
Please, do not use downstream naming and conventions.

> + compatible = "nvmem-reboot-mode";
> + nvmem-cells = <&reboot_reason>;
> + nvmem-cell-names = "reboot-mode";
> + mode-recovery = <0x01>;
> + mode-bootloader = <0x02>;
> + };
> };
Best regards,
Krzysztof

2023-04-16 15:16:56

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/4] arm64: defconfig: enable building the nvmem-reboot-mode module

On 13/04/2023 15:17, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> This module is used by the Qualcomm sa8775p platform for passing the
> reboot reason to the bootloader. Enable building it in the arm64
> defconfig as a module.
>
> Signed-off-by: Bartosz Golaszewski <[email protected]>

Reviewed-by: Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof

2023-04-16 15:31:57

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 3/4] dt-bindings: power: reset: convert nvmem-reboot-mode bindings to YAML

On 13/04/2023 15:17, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> Convert the DT binding document for nvmem-reboot-mode from .txt to YAML.

Thank you for your patch. There is something to discuss/improve.

> diff --git a/Documentation/devicetree/bindings/power/reset/nvmem-reboot-mode.yaml b/Documentation/devicetree/bindings/power/reset/nvmem-reboot-mode.yaml
> new file mode 100644
> index 000000000000..64a7d224c7dd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/reset/nvmem-reboot-mode.yaml
> @@ -0,0 +1,52 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/reset/nvmem-reboot-mode.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic NVMEM reboot mode driver

Drop "driver".
> +
> +maintainers:
> + - Bartosz Golaszewski <[email protected]>
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> + This driver gets the reboot mode magic value from the reboot-mode driver
> + and stores it in the NVMEM cell named "reboot-mode". The bootloader can
> + then read it and take different action according to the value.
> +
> +properties:
> + compatible:
> + const: nvmem-reboot-mode
> +
> + nvmem-cells:
> + description: |

Do not need '|' unless you need to preserve formatting.

> + A phandle pointing to the nvmem-cells node where the vendor-specific
> + magic value representing the reboot mode is stored.
> + maxItems: 1
> +
> + nvmem-cell-names:
> + items:
> + - const: reboot-mode
> +
> +patternProperties:
> + "^mode-.+":
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: Vendor-specific mode value written to the mode register
> +
> +additionalProperties: false
> +
> +required:
> + - compatible
> + - nvmem-cells
> + - nvmem-cell-names

put required: before additionalProperties

Reviewed-by: Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof

2023-04-17 14:42:58

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 2/4] arm64: defconfig: enable building the nvmem-reboot-mode module

On Sun, Apr 16, 2023, at 17:16, Krzysztof Kozlowski wrote:
> On 13/04/2023 15:17, Bartosz Golaszewski wrote:
>> From: Bartosz Golaszewski <[email protected]>
>>
>> This module is used by the Qualcomm sa8775p platform for passing the
>> reboot reason to the bootloader. Enable building it in the arm64
>> defconfig as a module.
>>
>> Signed-off-by: Bartosz Golaszewski <[email protected]>
>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>

Applied, thanks!

Arnd

2023-04-17 14:44:09

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 0/4] arm64: qcom: fix the reboot reason handling on sa8775p

On Thu, Apr 13, 2023, at 15:17, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> SA8775P uses nvmem to pass the reboot reason magic value to the bootloader.
> Remove the reboot modes from the PON node and introduce an SDAM node passed
> to the nvmem-reboot-mode driver. While at it: convert the bindings for
> nvmem-reboot-mode to YAML and enable it for arm64 in defconfig.
>
> Bartosz Golaszewski (2):
> arm64: defconfig: enable building the nvmem-reboot-mode module
> dt-bindings: power: reset: convert nvmem-reboot-mode bindings to YAML
>
> Parikshit Pareek (2):
> arm64: dts: qcom: sa8775p: pmic: remove the PON modes
> arm64: dts: qcom: sa8775p: pmic: add the sdam_0 node

It looks like it's too late for the dts patches in v6.4, but I was
picking up defconfig patches from the list, and that one seems useful
regardless of the rest, so I added it to the soc/defconfig branch.

Arnd

2023-04-18 04:47:02

by Shazad Hussain

[permalink] [raw]
Subject: Re: [PATCH 1/4] arm64: dts: qcom: sa8775p: pmic: remove the PON modes



On 4/13/2023 9:42 PM, Krzysztof Kozlowski wrote:
> On 13/04/2023 15:17, Bartosz Golaszewski wrote:
>> From: Parikshit Pareek <[email protected]>
>>
>> Remove the power on reasons with reboot from the pmm8654au_0_pon.
>> Instead, the PoN reaons should be part of different sdam_0 mode, to
>
> typo: reasons
>
>> be interoduced.
>
> introduced
>
> Anyway it does not say why. Are these power reasons not correct?
>

Hi Krzysztof,
Since sm8350 the PMIC PON peripheral was split into PON_HLOS and PON_PBS
to avoid security concerns with HLOS APPS being able to trigger a PMIC
WARM_RESET unilaterally. When the split occurred, the spare registers
ended up in PON_PBS, not PON_HLOS. Thus at that time, we moved to using
an SDAM register for Linux “reboot reason” configuration. And bootloader
also SDAM register to get these reboot region data to get into
bootloader/edl, so to have this working we need to use SDAM.

>>
>> Signed-off-by: Parikshit Pareek <[email protected]>
>> Signed-off-by: Bartosz Golaszewski <[email protected]>
>> ---
>
> Best regards,
> Krzysztof
>

-Shazad

2023-04-18 07:15:08

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/4] arm64: dts: qcom: sa8775p: pmic: remove the PON modes

On 18/04/2023 06:39, Shazad Hussain wrote:
>
>
> On 4/13/2023 9:42 PM, Krzysztof Kozlowski wrote:
>> On 13/04/2023 15:17, Bartosz Golaszewski wrote:
>>> From: Parikshit Pareek <[email protected]>
>>>
>>> Remove the power on reasons with reboot from the pmm8654au_0_pon.
>>> Instead, the PoN reaons should be part of different sdam_0 mode, to
>>
>> typo: reasons
>>
>>> be interoduced.
>>
>> introduced
>>
>> Anyway it does not say why. Are these power reasons not correct?
>>
>
> Hi Krzysztof,
> Since sm8350 the PMIC PON peripheral was split into PON_HLOS and PON_PBS
> to avoid security concerns with HLOS APPS being able to trigger a PMIC
> WARM_RESET unilaterally. When the split occurred, the spare registers
> ended up in PON_PBS, not PON_HLOS. Thus at that time, we moved to using
> an SDAM register for Linux “reboot reason” configuration. And bootloader
> also SDAM register to get these reboot region data to get into
> bootloader/edl, so to have this working we need to use SDAM.

The explanation of their correctness should be in commit msg.

Best regards,
Krzysztof

2023-04-18 09:47:49

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH 1/4] arm64: dts: qcom: sa8775p: pmic: remove the PON modes



On 18.04.2023 06:39, Shazad Hussain wrote:
>
>
> On 4/13/2023 9:42 PM, Krzysztof Kozlowski wrote:
>> On 13/04/2023 15:17, Bartosz Golaszewski wrote:
>>> From: Parikshit Pareek <[email protected]>
>>>
>>> Remove the power on reasons with reboot from the pmm8654au_0_pon.
>>> Instead, the PoN reaons should be part of different sdam_0 mode, to
>>
>> typo: reasons
>>
>>> be interoduced.
>>
>> introduced
>>
>> Anyway it does not say why. Are these power reasons not correct?
>>
>
> Hi Krzysztof,
> Since sm8350 the PMIC PON peripheral was split into PON_HLOS and PON_PBS
> to avoid security concerns with HLOS APPS being able to trigger a PMIC
> WARM_RESET unilaterally. When the split occurred, the spare registers
> ended up in PON_PBS, not PON_HLOS. Thus at that time, we moved to using
> an SDAM register for Linux “reboot reason” configuration. And bootloader
> also SDAM register to get these reboot region data to get into
> bootloader/edl, so to have this working we need to use SDAM.
>
Does that imply all PMICs following the PMK8350 scheme (separate HLOS and
PBS) should direct reboot mode writes to SDAM?

Konrad
>>>
>>> Signed-off-by: Parikshit Pareek <[email protected]>
>>> Signed-off-by: Bartosz Golaszewski <[email protected]>
>>> ---
>>
>> Best regards,
>> Krzysztof
>>
>
> -Shazad

2023-04-18 09:56:40

by Shazad Hussain

[permalink] [raw]
Subject: Re: [PATCH 1/4] arm64: dts: qcom: sa8775p: pmic: remove the PON modes



On 4/18/2023 3:13 PM, Konrad Dybcio wrote:
>
>
> On 18.04.2023 06:39, Shazad Hussain wrote:
>>
>>
>> On 4/13/2023 9:42 PM, Krzysztof Kozlowski wrote:
>>> On 13/04/2023 15:17, Bartosz Golaszewski wrote:
>>>> From: Parikshit Pareek <[email protected]>
>>>>
>>>> Remove the power on reasons with reboot from the pmm8654au_0_pon.
>>>> Instead, the PoN reaons should be part of different sdam_0 mode, to
>>>
>>> typo: reasons
>>>
>>>> be interoduced.
>>>
>>> introduced
>>>
>>> Anyway it does not say why. Are these power reasons not correct?
>>>
>>
>> Hi Krzysztof,
>> Since sm8350 the PMIC PON peripheral was split into PON_HLOS and PON_PBS
>> to avoid security concerns with HLOS APPS being able to trigger a PMIC
>> WARM_RESET unilaterally. When the split occurred, the spare registers
>> ended up in PON_PBS, not PON_HLOS. Thus at that time, we moved to using
>> an SDAM register for Linux “reboot reason” configuration. And bootloader
>> also SDAM register to get these reboot region data to get into
>> bootloader/edl, so to have this working we need to use SDAM.
>>
> Does that imply all PMICs following the PMK8350 scheme (separate HLOS and
> PBS) should direct reboot mode writes to SDAM?
>
> Konrad

Yes, that's what the expectation is with bootloader using SDAM as well.

>>>>
>>>> Signed-off-by: Parikshit Pareek <[email protected]>
>>>> Signed-off-by: Bartosz Golaszewski <[email protected]>
>>>> ---
>>>
>>> Best regards,
>>> Krzysztof
>>>
>>
>> -Shazad

-Shazad