Re: [PATCH v9 20/23] scsi: ufs: mediatek: Back up idle timer in per-instance struct

["Chaotian Jing (井朝天)" <Chaotian.Jing@mediatek.com>]

On Fri, 2026-03-06 at 14:25 +0100, Nicolas Frattaroli wrote:
> The MediaTek UFS driver uses a function-scope static variable to back
> up
> a hardware register across a power change in the
> ufs_mtk_pwr_change_notify function. This is dangerous, as it's only
> correct if only ever one instance of the driver is loaded, which
> isn't
> true if there's more than one device on a SoC that needs it, or it
> otherwise gets loaded a second time.
> 
> Back it up into a member of the host struct instead, as this struct
> is
> per-instance. Rework the function to not use a pointless "ret" local
> as
> well.
> 
> Fixes: f5ca8d0c7a63 ("scsi: ufs: host: mediatek: Disable auto-hibern8 
> during power mode changes")
> Reviewed-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
Reviewed-by: Chaotian Jing <chaotian.jing@mediatek.com>
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>  drivers/ufs/host/ufs-mediatek.c | 20 ++++++++------------
>  drivers/ufs/host/ufs-mediatek.h |  1 +
>  2 files changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-
> mediatek.c
> index c4e70fb99e82..2198271a269a 100644
> --- a/drivers/ufs/host/ufs-mediatek.c
> +++ b/drivers/ufs/host/ufs-mediatek.c
> @@ -1398,28 +1398,24 @@ static int ufs_mtk_pwr_change_notify(struct
> ufs_hba *hba,
>  				const struct ufs_pa_layer_attr
> *dev_max_params,
>  				struct ufs_pa_layer_attr
> *dev_req_params)
>  {
> -	int ret = 0;
> -	static u32 reg;
> +	struct ufs_mtk_host *host = ufshcd_get_variant(hba);
>  
>  	switch (stage) {
>  	case PRE_CHANGE:
>  		if (ufshcd_is_auto_hibern8_supported(hba)) {
> -			reg = ufshcd_readl(hba,
> REG_AUTO_HIBERNATE_IDLE_TIMER);
> +			host->ahit = ufshcd_readl(
> +				hba, REG_AUTO_HIBERNATE_IDLE_TIMER);
>  			ufs_mtk_auto_hibern8_disable(hba);
>  		}
> -		ret = ufs_mtk_pre_pwr_change(hba, dev_max_params,
> -					     dev_req_params);
> -		break;
> +		return ufs_mtk_pre_pwr_change(hba, dev_max_params,
> dev_req_params);
>  	case POST_CHANGE:
>  		if (ufshcd_is_auto_hibern8_supported(hba))
> -			ufshcd_writel(hba, reg,
> REG_AUTO_HIBERNATE_IDLE_TIMER);
> -		break;
> -	default:
> -		ret = -EINVAL;
> -		break;
> +			ufshcd_writel(hba, host->ahit,
> +				      REG_AUTO_HIBERNATE_IDLE_TIMER);
> +		return 0;
>  	}
>  
> -	return ret;
> +	return -EINVAL;
>  }
>  
>  static int ufs_mtk_unipro_set_lpm(struct ufs_hba *hba, bool lpm)
> diff --git a/drivers/ufs/host/ufs-mediatek.h b/drivers/ufs/host/ufs-
> mediatek.h
> index fa27ab4d6d6c..2349d9b9375c 100644
> --- a/drivers/ufs/host/ufs-mediatek.h
> +++ b/drivers/ufs/host/ufs-mediatek.h
> @@ -187,6 +187,7 @@ struct ufs_mtk_host {
>  	u16 ref_clk_gating_wait_us;
>  	u32 ip_ver;
>  	bool legacy_ip_ver;
> +	u32 ahit;
>  
>  	bool mcq_set_intr;
>  	bool is_mcq_intr_enabled;
>