Re: [PATCH v3 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data

On 8/18/26 11:33, Aiden Isik wrote:
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>> - [Medium] Missing PMU counter enable configuration for Exynos5515 watchdog.
>> --
>>
>> --- Patch [2]: [PATCH v3 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data ---
>> commit 9ce360e3c933b7447ba5da9ba1b7ae3fee989703
>> Author: Aiden Isik <[email protected]>
>>
>> watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data
>>
>> This commit adds driver data for the Exynos5515 SoC's watchdog timer.
>>
>>> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
>>> index e31f93db05096..5592f25fe6c83 100644
>>> --- a/drivers/watchdog/s3c2410_wdt.c
>>> +++ b/drivers/watchdog/s3c2410_wdt.c
>>> @@ -232,6 +232,16 @@ static const struct s3c2410_wdt_variant drv_data_exynos5420 = {
>>>   		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_AUTO_DISABLE,
>>>   };
>>>
>>> +static const struct s3c2410_wdt_variant drv_data_exynos5515 = {
>>> +	.mask_reset_reg = EXYNOSAUTOV920_CLUSTER0_NONCPU_INT_EN,
>>> +	.mask_bit = 2,
>>> +	.mask_reset_inv = true,
>>> +	.rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
>>> +	.rst_stat_bit = 24,
>>> +	.quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET | \
>>> +		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_DBGACK_BIT,
>>> +};
>>
>> [Severity: Medium]
>> Does the Exynos5515 PMU require explicit counter enablement like other recent
>> Exynos SoCs?
>>
>> Other variants in this driver (like Exynos850, AutoV920, and GS101) that use
>> the NONCPU_INT_EN PMU registers also pair it with a NONCPU_OUT register and
>> the QUIRK_HAS_PMU_CNT_EN flag to ensure the watchdog counter ticks.
>>
>> If the Exynos5515 hardware works similarly, omitting cnt_en_reg, cnt_en_bit,
>> and QUIRK_HAS_PMU_CNT_EN here might leave the counter disabled and the
>> watchdog silently non-functional. Could you confirm if this variant needs the
>> counter enable logic?
> 
> Exynos5515 does not require this.
> 
> Downstream on GS101, as an example, has this struct member set:
> 
>> pmu_count_en_func
> 
> This is not set downstream for Exynos5515, so it is not used for
> Exynos5515.
> 

You keep talking about "downstream". What is that ?

Guenter