Re: [PATCH v10 24/26] arm64: dts: mediatek: mt8195: Align ufshci node to dt-bindings changes

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 <[email protected]>

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/[email protected]?part=24