Re: [PATCH v3 4/8] dt-bindings: watchdog: Add compatible for MediaTek mt8167

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `mediatek,mt8167-wdt` compatible string is introduced as a standalone compatible in the bindings but is not added to the watchdog driver's match table.
- [Medium] The commit message claims the reset bit constants are different for mt8167, but the corresponding dt-bindings header file is missing.
--

--- Patch [4]: [PATCH v3 4/8] dt-bindings: watchdog: Add compatible for MediaTek mt8167 ---
commit 801a3249de93e217207cdaff898bd46c2e41daba
Author: Luca Leonardo Scorcia <[email protected]>

dt-bindings: watchdog: Add compatible for MediaTek mt8167

This commit adds a standalone compatible string mediatek,mt8167-wdt to the
MediaTek watchdog binding. The commit message notes that mt8167 has different
reset lines compared to the mt6589 fallback.

>     Currently mt8167 inherits its watchdog from the included mt8516 dtsi,
>     which in turn falls back on mt6589-wdt. However according to the data
>     sheet mt8167 has 15 sw resets instead of 12, and the systems to which
>     each reset bit corresponds are different from mt6589, so the reset bit
>     constants are different too.

[Severity: Medium]
The commit message mentions that the reset bit constants are different, but
this commit does not seem to include the corresponding dt-bindings header file
(e.g., include/dt-bindings/reset/mt8167-resets.h). Are these constants defined
in another patch in this series, or were they inadvertently omitted, leaving
device tree authors unable to utilize the MT8167 reset controller?

> diff --git a/Documentation/devicetree/bindings/watchdog/mediatek,mtk-wdt.yaml b/Documentation/devicetree/bindings/watchdog/mediatek,mtk-wdt.yaml
> index 953629cb9558b..f514be8a5851d 100644
> --- a/Documentation/devicetree/bindings/watchdog/mediatek,mtk-wdt.yaml
> +++ b/Documentation/devicetree/bindings/watchdog/mediatek,mtk-wdt.yaml
> @@ -26,6 +26,7 @@ properties:
>            - mediatek,mt6795-wdt
>            - mediatek,mt7986-wdt
>            - mediatek,mt7988-wdt
> +          - mediatek,mt8167-wdt

[Severity: Medium]
Does adding this as a standalone compatible string without a fallback cause a
regression in probe behavior? 

Looking at drivers/watchdog/mtk_wdt.c, the mtk_wdt_dt_ids match table does
not appear to have an entry for mediatek,mt8167-wdt:

drivers/watchdog/mtk_wdt.c:mtk_wdt_dt_ids[] {
    { .compatible = "mediatek,mt2712-wdt", .data = &mt2712_data },
    ...
};

If a device tree is updated to use this new compatible string as specified by
this binding, will the kernel driver fail to probe, breaking watchdog and reset
functionality? Is a corresponding driver patch missing, or should this binding
retain a fallback compatible?

>            - mediatek,mt8183-wdt
>            - mediatek,mt8186-wdt
>            - mediatek,mt8188-wdt

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4