Re: [PATCH 2/5] power: reset: qcom-pon: Add support for WARM reset

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 11/12/25 12:16 PM, Loic Poulain wrote:
> Hi Konrad,
> 
> On Thu, Nov 6, 2025 at 1:50 PM Konrad Dybcio
> <konrad.dybcio@oss.qualcomm.com> wrote:
>>
>> On 11/5/25 10:44 PM, Loic Poulain wrote:
>>> Hi Konrad,
>>>
>>> On Tue, Nov 4, 2025 at 4:20 PM Konrad Dybcio
>>> <konrad.dybcio@oss.qualcomm.com> wrote:
>>>>
>>>> On 11/4/25 4:01 PM, Loic Poulain wrote:
>>>>> Hi Konrad, Krzysztof,
>>>>>
>>>>> On Tue, Nov 4, 2025 at 12:50 PM Konrad Dybcio
>>>>> <konrad.dybcio@oss.qualcomm.com> wrote:
>>>>>>
>>>>>> On 11/3/25 7:20 PM, Loic Poulain wrote:
>>>>>>> This mechanism can be used when firmware lacks proper warm-reset handling,
>>>>>>> for example, when the PSCI SYSTEM_RESET2 function is not implemented.
>>>>>>> It enables the warm reset functionality via the PMIC.
>>>>>>>
>>>>>>> This fallback is only enabled if qcom,warm-reset property is present.
>>>>>>>
>>>>>>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
>>>>>>> ---
>>>>>>>  drivers/power/reset/qcom-pon.c | 47 ++++++++++++++++++++++++++++++++++
>>>>>>>  1 file changed, 47 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/power/reset/qcom-pon.c b/drivers/power/reset/qcom-pon.c
>>>>>>> index 7e108982a582..684e9fe9987d 100644
>>>>>>> --- a/drivers/power/reset/qcom-pon.c
>>>>>>> +++ b/drivers/power/reset/qcom-pon.c
>>>>>>> @@ -19,12 +19,20 @@
>>>>>>>
>>>>>>>  #define NO_REASON_SHIFT                      0
>>>>>>>
>>>>>>> +#define PON_SW_RESET_S2_CTL                          0x62
>>>>>>> +#define              PON_SW_RESET_S2_CTL_WARM_RST    0x01
>>>>>>> +#define PON_SW_RESET_S2_CTL2                         0x63
>>>>>>> +#define              PON_SW_RESET_S2_CTL2_RST_EN     BIT(7)
>>>>>>> +#define PON_SW_RESET_GO                                      0x64
>>>>>>> +#define              PON_SW_RESET_GO_MAGIC           0xa5
>>>>>>
>>>>>> Going back to msm8974 where the SPMI arbiter first showed up, these
>>>>>> values are all seemingly valid, so I think we can drop the dt property.
>>>>>> The restart reasons are set in stone too, and you can find more of them
>>>>>> in the register description.
>>>>>
>>>>> Yes, but this should only apply when the platform firmware does not
>>>>> support warm reset via PSCI, right?
>>>>> Making it unconditional would override the PSCI implementation even
>>>>> when warm reset is supported.
>>>>>
>>>>> The point is that psci_sys_reset() executes a cold reset if warm
>>>>> reset isn’t available. Therefore, our PMIC reboot notifier must have a
>>>>> higher priority than the PSCI handler.
>>>>>
>>>>> So maybe the alternative could be to introduce an additional reboot
>>>>> handler in psci, with the lowest priority, so that warm reset can have
>>>>> a chance to run either from the psci main reboot handler or from the
>>>>> PMIC reboot handler before falling back to cold reset?
>>>>> [PSCI-handler]->[other-handlers]->[PSCI-cold-reset-fallback-handler]
>>>>
>>>> This seems like a common enough problem, perhaps the framework could
>>>> accept EOPNOTSUPP or similar and try to delegate further, coming back
>>>> with a normal restart or so, if unsupported. Trying to make a special
>>>> contract between qcom-pon and psci silently will be very fragile
>>>> otherwise.
>>>
>>> I tested the following, as described above:
>>> https://github.com/loicpoulain/linux/commit/5c34ea54e1a21ff1192c3c341877b24eff5f80b4
>>> The only special 'contract' is the handler priority.
>>> If you can elaborate on another/better approach, that would be helpful.
>>
>> Thinking about it again, it'd be difficult to grab some sort of a handle
>> to the ""parent"" reboot mode, so what you propose here is good
>>
>>>>>> That said, we're circumventing PS_HOLD this way - is that intended?
>>>>>
>>>>> Well, we don’t have direct control over PS_HOLD since it’s managed by
>>>>> the firmware in our case. That’s why I considered using the PMIC
>>>>> software reset as an effective way to achieve this warm reset.
>>>>
>>>> Hm, so is there no longer a way to assert it by writing to PMIC
>>>> registers?
>>>
>>> PS_HOLD is a SoC signal, and we can maybe assert it via the
>>> MPM_PS_HOLD register through the msm-poweroff driver if needed (well,
>>> if access is allowed from a non-secure world).
>>> However, this would also require coordination with the PMIC driver to
>>> select the correct PS_HOLD action (shutdown, cold reset, warm reset).
>>> For that reason, I’d prefer to keep PS_HOLD based logic abstracted by PSCI.
>>> Using the SW_RST PMIC register allows us to perform a reset without
>>> additional signal handling.
>>
>> Yeah of course we should use PSCI where functional and available
>>
>> I think PS_HOLD used to be fully manual on old (msm-3.10) platforms
>> through PMIC registers. I see that e.g. msm-4.19 has an SCM call to
>> (de)assert it. There's also a "halt PMIC arbiter" call.
>>
>> (via drivers/power/reset/msm-poweroff.c)
> 
> Yes I could try the SCM call to deassert PS_HOLD, is it something we
> should prefer over PMIC soft reset?
> Asking because the implication would be a more complex solution
> (though not yet tested):
> - Adding reboot mode support in qcom-scm to activate ELD mode
> - Adding reset-notifier in pmic driver to modify PS_HOLD action to warm-reset
> - Adding reset-notifier in qcom,scm (of lower priority than PMIC)
> doing the actual SCM ps-hold deassert
> - Ensuring that PSCI is still used for cold-reset and warm-reset when
> supported...

My answer is unfortunately "I don't know". We should loop in some
PMIC folks that would know the difference

Konrad