[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7d719385-beee-4780-ac6b-8c5cace90b1e@acm.org>
Date: Fri, 24 May 2024 06:56:27 -0700
From: Bart Van Assche <bvanassche@....org>
To: Peter Wang (王信友) <peter.wang@...iatek.com>,
"hch@...radead.org" <hch@...radead.org>,
"Avri.Altman@....com" <Avri.Altman@....com>
Cc: "beanhuo@...ron.com" <beanhuo@...ron.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
"martin.petersen@...cle.com" <martin.petersen@...cle.com>
Subject: Re: [PATCH v5 2/3] scsi: ufs: Allow platform vendors to set rtt
On 5/23/24 23:06, Peter Wang (王信友) wrote:
> diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-
> mediatek.c
> index c4f997196c57..f8725f3374f7 100644
> --- a/drivers/ufs/host/ufs-mediatek.c
> +++ b/drivers/ufs/host/ufs-mediatek.c
> @@ -1777,6 +1777,32 @@ static int ufs_mtk_config_esi(struct ufs_hba
> *hba)
> return ufs_mtk_config_mcq(hba, true);
> }
>
> +static void ufs_mtk_set_rtt(struct ufs_hba *hba)
> +{
> + struct ufs_dev_info *dev_info = &hba->dev_info;
> + u32 rtt = 0;
> + u32 dev_rtt = 0;
> +
> + /* RTT override makes sense only for UFS-4.0 and above */
> + if (dev_info->wspecversion < 0x400)
> + return;
> +
> + if (ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
> + QUERY_ATTR_IDN_MAX_NUM_OF_RTT, 0,
> 0, &dev_rtt)) {
> + dev_err(hba->dev, "failed reading bMaxNumOfRTT\n");
> + return;
> + }
> +
> + /* override if not mediatek support */
> + if (dev_rtt == MTK_MAX_NUM_RTT)
> + return;
> +
> + rtt = MTK_MAX_NUM_RTT;
> + if (ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
> + QUERY_ATTR_IDN_MAX_NUM_OF_RTT, 0,
> 0, &rtt))
> + dev_err(hba->dev, "failed writing bMaxNumOfRTT\n");
> +}
> +
> /*
> * struct ufs_hba_mtk_vops - UFS MTK specific variant operations
> *
> @@ -1805,6 +1831,7 @@ static const struct ufs_hba_variant_ops
> ufs_hba_mtk_vops = {
> .op_runtime_config = ufs_mtk_op_runtime_config,
> .mcq_config_resource = ufs_mtk_mcq_config_resource,
> .config_esi = ufs_mtk_config_esi,
> + .set_rtt = ufs_mtk_set_rtt,
> };
>
> /**
> diff --git a/drivers/ufs/host/ufs-mediatek.h b/drivers/ufs/host/ufs-
> mediatek.h
> index 3ff17e95afab..05d76a6bd772 100644
> --- a/drivers/ufs/host/ufs-mediatek.h
> +++ b/drivers/ufs/host/ufs-mediatek.h
> @@ -189,4 +189,7 @@ struct ufs_mtk_host {
> /* MTK delay of autosuspend: 500 ms */
> #define MTK_RPM_AUTOSUSPEND_DELAY_MS 500
>
> +/* MTK RTT support number */
> +#define MTK_MAX_NUM_RTT 2
> +
> #endif /* !_UFS_MEDIATEK_H */
The above patch would result in duplication of code. We should avoid
code duplication if that's reasonably possible. Instead of applying the
above patch, I propose to add a callback function pointer in struct
ufs_hba_variant_ops that returns the maximum RTT value supported by the
host driver.
Thanks,
Bart.
Powered by blists - more mailing lists