Devicetree
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Louis-Alexis Eyraud" <louisalexis.eyraud@collabora.com>
Cc: olteanv@gmail.com, robh@kernel.org, devicetree@vger.kernel.org,
	conor+dt@kernel.org, linux-scsi@vger.kernel.org,
	neil.armstrong@linaro.org, linux-phy@lists.infradead.org,
	vkoul@kernel.org
Subject: Re: [PATCH v10 24/26] arm64: dts: mediatek: mt8195: Align ufshci node to dt-bindings changes
Date: Thu, 30 Jul 2026 11:13:26 +0000	[thread overview]
Message-ID: <20260730111326.E2D051F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <20260730-mt8196-ufs-v10-24-bc5ecef202dc@collabora.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing the `freq-table-hz` property without adding an `operating-points-v2` table causes the UFS core platform driver to skip parsing and enabling all hardware clocks, resulting in a controller initialization failure.
--

--- Patch [24]: [PATCH v10 24/26] arm64: dts: mediatek: mt8195: Align ufshci node to dt-bindings changes ---
commit eac3ed6e6791c53424e4e025e4dfd47b68309eb7
Author: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>

arm64: dts: mediatek: mt8195: Align ufshci node to dt-bindings changes

This commit updates the MT8195 UFS host controller dt bindings by adding
new required clocks and removing the deprecated freq-table-hz property.
It states that converting to an OPP table is not needed since the values
were all zero.

> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
[ ... ]
> @@ -1481,15 +1480,15 @@ ufshci: ufshci@11270000 {
>  				 <&infracfg_ao CLK_INFRA_AO_UNIPRO_TICK>,
>  				 <&infracfg_ao CLK_INFRA_AO_UFS_MP_SAP_B>,
>  				 <&infracfg_ao CLK_INFRA_AO_UFS_TX_SYMBOL>,
> -				 <&infracfg_ao CLK_INFRA_AO_PERI_UFS_MEM_SUB>;
> -			clock-names = "ufs", "ufs_aes", "ufs_tick",
> -					"unipro_sysclk", "unipro_tick",
> -					"unipro_mp_bclk", "ufs_tx_symbol",
> -					"ufs_mem_sub";
> -			freq-table-hz = <0 0>, <0 0>, <0 0>,
> -					<0 0>, <0 0>, <0 0>,
> -					<0 0>, <0 0>;
> -

[Severity: High]
Does removing this property introduce a regression in the UFS controller
initialization?

Looking at ufshcd_parse_clock_info() in drivers/ufs/host/ufshcd-pltfrm.c,
the driver skips parsing the clock properties entirely if freq-table-hz is
missing:

drivers/ufs/host/ufshcd-pltfrm.c:ufshcd_parse_clock_info() {
    ...
	sz = of_property_count_u32_elems(np, "freq-table-hz");
	if (sz <= 0) {
		dev_info(dev, "freq-table-hz property not specified\n");
		goto out;
	}
    ...
}

Since an operating-points-v2 node is also absent, the alternative parser
ufshcd_parse_operating_points() will also return early. This leaves
hba->clk_list_head empty.

If the list is empty, ufshcd_init_clocks() and ufshcd_setup_clocks()
will silently return without turning on any hardware clocks. 

Could this cause a probe failure or a system crash (SError) when the driver
later attempts to access unclocked UFS MMIO registers?

> +				 <&infracfg_ao CLK_INFRA_AO_PERI_UFS_MEM_SUB>,
> +				 <&topckgen CLK_TOP_UFS_RX_SYMBOL>,
> +				 <&topckgen CLK_TOP_UFS_RX_SYMBOL1>;
> +			clock-names = "main", "aes", "tick", "unipro_sysclk",
> +				      "unipro_tick", "unipro_mp_bclk",
> +				      "tx_symbol", "mem_sub", "rx_symbol0",
> +				      "rx_symbol1";
> +			interrupts = <GIC_SPI 137 IRQ_TYPE_LEVEL_HIGH 0>;
> +			phys = <&ufsphy>;
>  			mediatek,ufs-disable-mcq;
>  			status = "disabled";
>  		};

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

  reply	other threads:[~2026-07-30 11:13 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
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 [this message]
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=20260730111326.E2D051F00A3A@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