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

Hi Guenter,

> On 8/18/26 12:35, Aiden Isik wrote:
> > Hi Guenter,
> >
> >> 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 ?
> >
> > By "downstream" I am referring specifically to Samsung's Linux kernel
> > fork for Exynos5515.
> >
> > I host a copy here: https://github.com/Aiden-Isik/linux-android-lucky7
> >
> > Also in this message I refer to Google's GS101 kernel fork, which can
> > be found here:
> > https://github.com/LineageOS/android_kernel_google_gs101
> >
> 
> Both are completely irrelevant for the upstream kernel. I am most definitely
> not going to look into that code. If there is anything relevant - such as
> the comment you pointed to earlier - you'll need to provide that as independent
> comment(s), not just refer to some downstream code as explanation. The
> explanation needs to be provided in the _upstream_ kernel code.
> 

There's not really any explanation for it other than "that is the way it is".
Samsung designed the Exynos5515 watchdog timer to not have that quirk,
I read the downstream code and learnt that fact, so we don't enable
that quirk.

> Guenter
> 
> >>
> >> Guenter
> >>
> >>
> >
> > Kind regards,
> > - Aiden Isik
> 
> 

Kind regards,
- Aiden Isik