Re: [PATCH 2/3] watchdog: Add Nuvoton MA35D1 watchdog driver support

Apologies for the broken formatting in my previous email due to a mail client configuration issue. Resending with proper inline formatting:

Guenter Roeck 於 2026/7/23 下午 10:07 寫道:
> On 7/22/26 23:20, [email protected] wrote:
>> Thank you for your contribution! Sashiko AI review found 9 potential issue(s) to consider:
>> - [High] The `ma35d1_wdt->dev` pointer is never initialized, leading to NULL pointer dereferences in logging functions.

Fix in v2. Switched to using wdt_dev->parent and direct device pointers for logging functions.Remove redundant ma35d1_wdt->dev.

>> - [High] Read-modify-write operations on REG_WDT_CTL inadvertently clear W1C (write-one-to-clear) status flags.

Not real.The W1C flags (IF/WKF) are only utilized by PM callbacks during the suspend/resume window for wake-up event handling. During runtime, clearing these flags via RMW in set_timeout causes no functional side-effects. Furthermore, RSTF is already consumed and cleared during probe().

>> - [High] Missing clock domain synchronization (`ma35d1_wdt_wait_sync`) after writes to `REG_WDT_CTL` can lead to dropped register writes.

Fix in v2.Add ma35d1_wdt_wait_sync() after writing WDTEN to REG_WDT_CTL in ma35d1_wdt_start().

>> - [High] Driver overwrites `wdt_dev->timeout`, breaking the software heartbeat extension.

Fix in v2

>> - [High] Missing cleanup for wake IRQ allocated by dev_pm_set_wake_irq(), leading to a memory leak and permanent device unbind/bind failures.

Fix in v2.switch to devm_pm_set_wake_irq() in v2.

>> - [High] The driver fails to check if the watchdog was already enabled by the bootloader, causing unexpected system reboots during boot.

Fix in v2.Added a check to set WDOG_HW_RUNNING when necessary.

>> - [Medium] Unsigned integer comparison against zero renders IRQ error handling dead code.

Fix in v2.Change the IRQ variable type to int in v2 to properly handle negative error codes.

>> - [Medium] Integer overflow in `target_ticks` calculation triggers unexpected system resets for large timeouts.

Fix in v2.Add a check to skip calculation for large timeouts.

>> - [Low] Unnecessary dependency: selects `MFD_SYSCON` without using it.

Fix in v2.remove select MFD_SYSCON.

> 
> Please fix or explain why the issues reported by Sashiko are not real.
> 
> Thanks,
> Guenter
> 

Regards,
Zi-Yu Chen