mbox series

[0/4] arm64: qcom: fix the reboot reason handling on sa8775p

Message ID 20230413131705.3073911-1-brgl@bgdev.pl
Headers show
Series arm64: qcom: fix the reboot reason handling on sa8775p | expand

Message

Bartosz Golaszewski April 13, 2023, 1:17 p.m. UTC
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

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

Comments

Krzysztof Kozlowski April 16, 2023, 3:16 p.m. UTC | #1
On 13/04/2023 15:17, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> 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 <bartosz.golaszewski@linaro.org>

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

Best regards,
Krzysztof
Arnd Bergmann April 17, 2023, 2:38 p.m. UTC | #2
On Sun, Apr 16, 2023, at 17:16, Krzysztof Kozlowski wrote:
> On 13/04/2023 15:17, Bartosz Golaszewski wrote:
>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>> 
>> 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 <bartosz.golaszewski@linaro.org>
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Applied, thanks!

     Arnd
Arnd Bergmann April 17, 2023, 2:42 p.m. UTC | #3
On Thu, Apr 13, 2023, at 15:17, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> 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
Shazad Hussain April 18, 2023, 4:39 a.m. UTC | #4
On 4/13/2023 9:42 PM, Krzysztof Kozlowski wrote:
> On 13/04/2023 15:17, Bartosz Golaszewski wrote:
>> From: Parikshit Pareek <quic_ppareek@quicinc.com>
>>
>> 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 <quic_ppareek@quicinc.com>
>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>> ---
> 
> Best regards,
> Krzysztof
> 

-Shazad
Krzysztof Kozlowski April 18, 2023, 7:04 a.m. UTC | #5
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 <quic_ppareek@quicinc.com>
>>>
>>> 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
Konrad Dybcio April 18, 2023, 9:43 a.m. UTC | #6
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 <quic_ppareek@quicinc.com>
>>>
>>> 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 <quic_ppareek@quicinc.com>
>>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>> ---
>>
>> Best regards,
>> Krzysztof
>>
> 
> -Shazad
Shazad Hussain April 18, 2023, 9:54 a.m. UTC | #7
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 <quic_ppareek@quicinc.com>
>>>>
>>>> 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 <quic_ppareek@quicinc.com>
>>>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>>> ---
>>>
>>> Best regards,
>>> Krzysztof
>>>
>>
>> -Shazad

-Shazad