Re: [PATCH v10 22/26] scsi: ufs: mediatek: Remove undocumented "clk-scale-up-vcore-min"

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Incomplete feature removal leaves behind unused struct field and dead code for UFS clock vcore scaling.
--

--- Patch [22]: [PATCH v10 22/26] scsi: ufs: mediatek: Remove undocumented "clk-scale-up-vcore-min" ---
commit 35c917bdf36a893b4dd563ccc122d8d25ec6d9ed
Author: Nicolas Frattaroli <[email protected]>

scsi: ufs: mediatek: Remove undocumented "clk-scale-up-vcore-min"

This commit removes support for the undocumented "clk-scale-up-vcore-min" 
property from the MediaTek UFS driver. It deletes the device tree parsing 
logic and the initial vcore regulator setup.

> diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
> index 90c8606d85a9d..5dd72b8261815 100644
> --- a/drivers/ufs/host/ufs-mediatek.c
> +++ b/drivers/ufs/host/ufs-mediatek.c

[ ... ]

> @@ -918,24 +916,7 @@ static void ufs_mtk_init_clocks(struct ufs_hba *hba)
>  	if (!ufs_mtk_is_clk_scale_ready(hba)) {
>  		hba->caps &= ~UFSHCD_CAP_CLK_SCALING;
>  		dev_info(hba->dev, "%s: Clock scaling unavailable", __func__);
> -		return;
> -	}
> -
> -	if (!host->reg_vcore)
> -		return;
> -
> -	if (of_property_read_u32(dev->of_node, "clk-scale-up-vcore-min",
> -				 &volt)) {
> -		dev_info(dev, "failed to get clk-scale-up-vcore-min");
> -		return;
>  	}
> -
> -	host->mclk.vcore_volt = volt;

[Severity: Low]
Since this patch removes the code that populates host->mclk.vcore_volt, does 
it leave behind an unused field and dead code elsewhere in the driver?

In drivers/ufs/host/ufs-mediatek.h:
struct ufs_mtk_clk {
        ...
        int vcore_volt;
        ...
};

And in drivers/ufs/host/ufs-mediatek.c:_ufs_mtk_clk_scale():

    volt = host->mclk.vcore_volt;
    if (host->reg_vcore && volt)
            clk_bind_vcore = true;

Because vcore_volt is zero-initialized and no longer updated, clk_bind_vcore 
appears to always be false, which makes the subsequent regulator scaling 
branches permanently unreachable. Should these leftovers be cleaned up as 
well?

> -
> -	/* If default boot is max gear, request vcore */
> -	if (volt && host->clk_scale_up)
> -		if (regulator_set_voltage(host->reg_vcore, volt, INT_MAX))
> -			dev_err(hba->dev, "Failed to set vcore to %d\n", volt);
>  }

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