All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Luca Leonardo Scorcia" <l.scorcia@gmail.com>
Cc: conor+dt@kernel.org, devicetree@vger.kernel.org, robh@kernel.org,
	linux-watchdog@vger.kernel.org
Subject: Re: [PATCH v3 4/8] dt-bindings: watchdog: Add compatible for MediaTek mt8167
Date: Sun, 09 Aug 2026 16:23:47 +0000	[thread overview]
Message-ID: <20260809162347.ADC541F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260809160643.33991-5-l.scorcia@gmail.com>

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 <l.scorcia@gmail.com>

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/20260809160643.33991-1-l.scorcia@gmail.com?part=4

  reply	other threads:[~2026-08-09 16:23 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-09 16:03 [PATCH v3 0/8] Properly describe mt6589 and mt8167 toprgu resets Luca Leonardo Scorcia
2026-08-09 16:03 ` [PATCH v3 1/8] dt-bindings: reset: Add mt6589 toprgu reset IDs Luca Leonardo Scorcia
2026-08-11  8:54   ` Krzysztof Kozlowski
2026-08-11 14:20     ` Luca Leonardo Scorcia
2026-08-09 16:03 ` [PATCH v3 2/8] watchdog: mediatek: Add wdt/toprgu resets for MT6589 Luca Leonardo Scorcia
2026-08-09 16:29   ` sashiko-bot
2026-08-09 16:03 ` [PATCH v3 3/8] arm: dts: mediatek: mt6589: Enable toprgu reset controller Luca Leonardo Scorcia
2026-08-09 17:04   ` Akari Tsuyukusa
2026-08-09 17:08     ` Luca Leonardo Scorcia
2026-08-09 17:16       ` Akari Tsuyukusa
2026-08-09 17:49     ` Guenter Roeck
2026-08-09 18:55       ` Luca Leonardo Scorcia
2026-08-09 19:09         ` Guenter Roeck
2026-08-09 16:03 ` [PATCH v3 4/8] dt-bindings: watchdog: Add compatible for MediaTek mt8167 Luca Leonardo Scorcia
2026-08-09 16:23   ` sashiko-bot [this message]
2026-08-09 16:03 ` [PATCH v3 5/8] dt-bindings: reset: Add reset controller constants for mt8167 Luca Leonardo Scorcia
2026-08-09 16:25   ` sashiko-bot
2026-08-11  8:55   ` Krzysztof Kozlowski
2026-08-09 16:03 ` [PATCH v3 6/8] watchdog: mediatek: Add support for mt8167 TOPRGU/WDT Luca Leonardo Scorcia
2026-08-09 16:03 ` [PATCH v3 7/8] arm64: dts: mt8167: Properly describe the SoC watchdog Luca Leonardo Scorcia
2026-08-09 16:03 ` [PATCH v3 8/8] soc: mediatek: mtk-mmsys: Add resets for mt8167 Luca Leonardo Scorcia

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260809162347.ADC541F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=l.scorcia@gmail.com \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.