Re: [PATCH v4 2/3] power: reset: pscrr: add watchdog pretimeout reason tracking

[Faruque Ansari <faruque.ansari@oss.qualcomm.com>]

Hi Daniel Lezcano,

On 07-Oct-26 4:24 PM, Daniel Lezcano wrote:
> On 8/5/26 10:54, Faruque Ansari wrote:
>> Watchdog pretimeout resets are not recorded with a dedicated 
>> reason,
>> causing subsequent boots to report PSCR_UNKNOWN and making it 
>> difficult
>> to distinguish them from other unexpected resets.
>>
>> Add PSCR_WATCHDOG_PRETIMEOUT as a dedicated reason code and 
>> prevent the
>> panic notifier from overwriting a watchdog pretimeout reason with
>> PSCR_KERNEL_PANIC when the pretimeout governor triggers a panic.
>>
>> Signed-off-by: Faruque Ansari <faruque.ansari@oss.qualcomm.com>
>> ---
>>   drivers/power/reset/pscrr/pscrr.c     | 11 +++++++----
>>   include/linux/power/power_on_reason.h |  1 +
>>   include/linux/reboot.h                |  4 ++++
>>   kernel/reboot.c                       |  1 +
>>   4 files changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/power/reset/pscrr/pscrr.c b/drivers/power/ 
>> reset/pscrr/pscrr.c
>> index 8c45b84059a9..b7e6239e207e 100644
>> --- a/drivers/power/reset/pscrr/pscrr.c
>> +++ b/drivers/power/reset/pscrr/pscrr.c
>> @@ -529,12 +529,15 @@ static struct notifier_block 
>> pscrr_reboot_nb = {
>>   static int pscrr_panic_notifier(struct notifier_block *nb,
>>                   unsigned long action, void *unused)
>>   {
>> -    struct pscrr_provider_dir *dir;
>> -    set_psc_reason(PSCR_KERNEL_PANIC);
>> +    /*
>> +     * Do not overwrite a watchdog pretimeout reason already 
>> set by the
>> +     * pretimeout path before it triggered this panic.
>> +     */
>> +    if (get_psc_reason() != PSCR_WATCHDOG_PRETIMEOUT)
>> +        set_psc_reason(PSCR_KERNEL_PANIC);
> 
> It does not sounds correct.
> 
> May be you should move the set_psc_reason() to different places 
> for panic.
> 
> I did not explore the panic() function but I would suggest to 
> have something like:
> 
> panic(..., ) --> defaults to PSCR_KERNEL_PANIC
> 
> panic_with_pscrr(..., reason)
> 
> Then pscrr_panic_notifier() only does the record

Thanks for your review.

I agree that handling the kernel panic reason in this notifier 
may not be the ideal approach today. However, my intention is to 
add userspace crash recording support in the future as well. 
Since this notifier already serves as the common point for 
recording restart reasons, it seems like the most appropriate 
place to handle userspace-triggered reboot reasons when that 
support is added.

For consistency, I added the handling for kernel panic and 
watchdog reset reasons here as well. While they are not strictly 
required for the current use case, this notifier is the most 
suitable place to record reboot reasons. Keeping all reboot 
reason handling in one place will also make it easier to add 
userspace reboot/crash recording support in the future, without 
introducing a separate mechanism later.


Thanks,
Faruque Ansari

> 
>> -    list_for_each_entry(dir, &pscrr_dirs, node)
>> -        pscrr_do_record(dir, get_psc_reason());
>> +    pscrr_record_current();
>>       return NOTIFY_DONE;
>>   }
>> diff --git a/include/linux/power/power_on_reason.h b/include/ 
>> linux/power/power_on_reason.h
>> index 8c99bdd75021..8e3889fa43ef 100644
>> --- a/include/linux/power/power_on_reason.h
>> +++ b/include/linux/power/power_on_reason.h
>> @@ -21,5 +21,6 @@
>>   #define POWER_ON_REASON_EC_PANIC "EC panic"
>>   #define POWER_ON_REASON_EXTERNAL "external reset"
>>   #define POWER_ON_REASON_KERNEL_PANIC "kernel panic"
>> +#define POWER_ON_REASON_WATCHDOG_PRETIMEOUT "watchdog pretimeout"
>>   #endif /* POWER_ON_REASON_H */
>> diff --git a/include/linux/reboot.h b/include/linux/reboot.h
>> index a117dd5eaecd..6d5ecc6cdc00 100644
>> --- a/include/linux/reboot.h
>> +++ b/include/linux/reboot.h
>> @@ -243,6 +243,9 @@ extern void orderly_reboot(void);
>>    *
>>    * @PSCR_KERNEL_PANIC: Reset that followed a kernel panic.
>>    *
>> + * @PSCR_WATCHDOG_PRETIMEOUT: Recorded when a watchdog 
>> pretimeout fires, before
>> + *  the pretimeout governor brings the system down.
>> + *
>>    * @PSCR_REASON_COUNT: Number of defined power state change 
>> reasons. This
>>    *    value is useful for range checking and potential future 
>> extensions
>>    *    while maintaining compatibility.
>> @@ -267,6 +270,7 @@ enum psc_reason {
>>       PSCR_CPU_CLK_FAIL,
>>       PSCR_XTAL_FAIL,
>>       PSCR_KERNEL_PANIC,
>> +    PSCR_WATCHDOG_PRETIMEOUT,
>>       /* Number of reasons */
>>       PSCR_REASON_COUNT,
>> diff --git a/kernel/reboot.c b/kernel/reboot.c
>> index 979ecf3b093c..fad4ef9681a6 100644
>> --- a/kernel/reboot.c
>> +++ b/kernel/reboot.c
>> @@ -1110,6 +1110,7 @@ static const struct psc_reason_desc 
>> psc_reason_descs[] = {
>>       [PSCR_CPU_CLK_FAIL]     = { "cpu-clock-failure", 
>> POWER_ON_REASON_CPU_CLK_FAIL },
>>       [PSCR_XTAL_FAIL]     = { "crystal-failure",      
>> POWER_ON_REASON_XTAL_FAIL },
>>       [PSCR_KERNEL_PANIC]     = { "kernel-panic",      
>> POWER_ON_REASON_KERNEL_PANIC },
>> +    [PSCR_WATCHDOG_PRETIMEOUT] = { "watchdog-pretimeout", 
>> POWER_ON_REASON_WATCHDOG_PRETIMEOUT },
>>   };
>>   /**
>>
>