Devicetree
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Louis-Alexis Eyraud" <louisalexis.eyraud@collabora.com>
Cc: devicetree@vger.kernel.org, linux-phy@lists.infradead.org,
	conor+dt@kernel.org, neil.armstrong@linaro.org, robh@kernel.org,
	vkoul@kernel.org, olteanv@gmail.com, linux-scsi@vger.kernel.org
Subject: Re: [PATCH v10 03/26] dt-bindings: ufs: mediatek,ufs: Add mt8196 variant
Date: Thu, 30 Jul 2026 09:59:58 +0000	[thread overview]
Message-ID: <20260730095958.88ED01F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260730-mt8196-ufs-v10-3-bc5ecef202dc@collabora.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] A typo in the `mediatek,mt8192-ufshci` conditional block uses `clocks-names` instead of `clock-names`, bypassing array length validation.
- [Low] The binding omits a supply property for the `AVDD09_UFS_1` pin, tying it to `AVDD09_UFS`, which violates the principle that Device Tree describes the SoC hardware rather than a specific board's power routing.
--

--- Patch [3]: [PATCH v10 03/26] dt-bindings: ufs: mediatek,ufs: Add mt8196 variant ---
commit e0eeeaec009272d40ddbc2fee8902782c15e776c
Author: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

dt-bindings: ufs: mediatek,ufs: Add mt8196 variant

The MT8196 SoC's UFS controller uses a different set of supplies, making it
incompatible with MT8195. This commit adds a compatible string for mt8196
and updates the binding. It also notes that the AVDD09_UFS and AVDD09_UFS_1
pins are commoned together on the referenced board.

> While it does have a AVDD09_UFS_1 pin in addition to the AVDD09_UFS pin,
> it appears that these two pins are commoned together, as the board
> schematic I have access to uses the same supply for both, and the
> downstream driver does not distinguish between the two supplies either.

[Severity: Low]
Does omitting a separate supply property for the AVDD09_UFS_1 pin violate the
principle that Device Tree bindings should describe the SoC hardware rather
than a specific board's power routing?

If a future board design supplies the AVDD09_UFS and AVDD09_UFS_1 pins from
separate voltage regulators, the current binding wouldn't be able to accurately
describe that hardware.

While reviewing the file touched by this patch, I noticed a regression in the
conditional block for the mt8192-ufshci compatible string:

Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml:allOf
>   - if:
>       properties:
>         compatible:
>           contains:
>             const: mediatek,mt8192-ufshci
>     then:
>       properties:
>         clocks:
>           minItems: 3
>           maxItems: 3
>         clocks-names:
>           minItems: 3
>           maxItems: 3

[Severity: Medium]
Is the "clocks-names" property a typo here? It seems like this should be
"clock-names".

Because of the typo, the length of the clock-names array is not properly
constrained for mt8192-ufshci. An invalid device tree with 3 clocks but a
mismatched number of clock-names could silently pass dt_binding_check validation.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730-mt8196-ufs-v10-0-bc5ecef202dc@collabora.com?part=3

  reply	other threads:[~2026-07-30  9:59 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-30  9:48 [PATCH v10 00/26] MediaTek UFS Cleanup and MT8196 Enablement Louis-Alexis Eyraud
2026-07-30  9:48 ` [PATCH v10 01/26] dt-bindings: phy: Add mediatek,mt8196-ufsphy variant Louis-Alexis Eyraud
2026-07-30  9:48 ` [PATCH v10 02/26] dt-bindings: ufs: mediatek,ufs: Complete the binding Louis-Alexis Eyraud
2026-07-30  9:57   ` sashiko-bot
2026-07-30  9:48 ` [PATCH v10 03/26] dt-bindings: ufs: mediatek,ufs: Add mt8196 variant Louis-Alexis Eyraud
2026-07-30  9:59   ` sashiko-bot [this message]
2026-07-30  9:49 ` [PATCH v10 04/26] scsi: ufs: mediatek: Move MTK_SIP_UFS_CONTROL to mtk_sip_svc.h Louis-Alexis Eyraud
2026-07-30  9:49 ` [PATCH v10 05/26] phy: mediatek: ufs: Add support for resets Louis-Alexis Eyraud
2026-07-30  9:49 ` [PATCH v10 06/26] scsi: ufs: mediatek: Rework resets Louis-Alexis Eyraud
2026-07-30 10:29   ` sashiko-bot
2026-07-30  9:49 ` [PATCH v10 07/26] scsi: ufs: mediatek: Rework 0.9V regulator Louis-Alexis Eyraud
2026-07-30 10:13   ` sashiko-bot
2026-07-30  9:49 ` [PATCH v10 08/26] scsi: ufs: mediatek: Rework init function Louis-Alexis Eyraud
2026-07-30 10:07   ` sashiko-bot
2026-07-30  9:49 ` [PATCH v10 09/26] scsi: ufs: mediatek: Rework the crypt-boost stuff Louis-Alexis Eyraud
2026-07-30 10:16   ` sashiko-bot
2026-07-30  9:49 ` [PATCH v10 10/26] scsi: ufs: mediatek: Handle misc host voltage regulators Louis-Alexis Eyraud
2026-07-30 10:29   ` sashiko-bot
2026-07-30  9:49 ` [PATCH v10 11/26] scsi: ufs: mediatek: Remove undocumented downstream reset cruft Louis-Alexis Eyraud
2026-07-30 10:23   ` sashiko-bot
2026-07-30  9:49 ` [PATCH v10 12/26] scsi: ufs: mediatek: Remove vendor kernel quirks cruft Louis-Alexis Eyraud
2026-07-30 10:30   ` sashiko-bot
2026-07-30  9:49 ` [PATCH v10 13/26] scsi: ufs: mediatek: Use the common PHY framework Louis-Alexis Eyraud
2026-07-30 10:40   ` sashiko-bot
2026-07-30  9:49 ` [PATCH v10 14/26] scsi: ufs: mediatek: Remove mediatek,ufs-broken-rtc property Louis-Alexis Eyraud
2026-07-30  9:49 ` [PATCH v10 15/26] scsi: ufs: mediatek: Rework _ufs_mtk_clk_scale error paths Louis-Alexis Eyraud
2026-07-30 10:34   ` sashiko-bot
2026-07-30  9:49 ` [PATCH v10 16/26] scsi: ufs: mediatek: Clean up logging prints Louis-Alexis Eyraud
2026-07-30  9:49 ` [PATCH v10 17/26] scsi: ufs: mediatek: Rework ufs_mtk_wait_idle_state Louis-Alexis Eyraud
2026-07-30  9:49 ` [PATCH v10 18/26] scsi: ufs: mediatek: Don't acquire dvfsrc-vcore twice Louis-Alexis Eyraud
2026-07-30 10:41   ` sashiko-bot
2026-07-30  9:49 ` [PATCH v10 19/26] scsi: ufs: mediatek: Rework hardware version reading Louis-Alexis Eyraud
2026-07-30  9:49 ` [PATCH v10 20/26] scsi: ufs: mediatek: Back up idle timer in per-instance struct Louis-Alexis Eyraud
2026-07-30 10:53   ` sashiko-bot
2026-07-30  9:49 ` [PATCH v10 21/26] scsi: ufs: mediatek: Remove ret local from link_startup_notify Louis-Alexis Eyraud
2026-07-30  9:49 ` [PATCH v10 22/26] scsi: ufs: mediatek: Remove undocumented "clk-scale-up-vcore-min" Louis-Alexis Eyraud
2026-07-30 10:49   ` sashiko-bot
2026-07-30  9:49 ` [PATCH v10 23/26] scsi: ufs: mediatek: Add MT8196 compatible, update copyright Louis-Alexis Eyraud
2026-07-30  9:49 ` [PATCH v10 24/26] arm64: dts: mediatek: mt8195: Align ufshci node to dt-bindings changes Louis-Alexis Eyraud
2026-07-30 11:13   ` sashiko-bot
2026-07-30  9:49 ` [PATCH v10 25/26] arm64: dts: mediatek: mt8395-genio-1200-evk-ufs: Complete UFS power supplies Louis-Alexis Eyraud
2026-07-30  9:49 ` [PATCH v10 26/26] arm64: dts: mediatek: mt8395-radxa-nio-12l: " Louis-Alexis Eyraud

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=20260730095958.88ED01F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-phy@lists.infradead.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=louisalexis.eyraud@collabora.com \
    --cc=neil.armstrong@linaro.org \
    --cc=olteanv@gmail.com \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=vkoul@kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox