Received-SPF: pass (google.com: domain of linux-kernel+bounces-188756-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <7d719385-beee-4780-ac6b-8c5cace90b1e@acm.org>
Date: Fri, 24 May 2024 06:56:27 -0700
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v5 2/3] scsi: ufs: Allow platform vendors to set rtt
To: =?UTF-8?B?UGV0ZXIgV2FuZyAo546L5L+h5Y+LKQ==?= <peter.wang@mediatek.com>,
 "hch@infradead.org" <hch@infradead.org>,
 "Avri.Altman@wdc.com" <Avri.Altman@wdc.com>
Cc: "beanhuo@micron.com" <beanhuo@micron.com>,
 "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
 "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
 "martin.petersen@oracle.com" <martin.petersen@oracle.com>
References: <20240523125827.818-1-avri.altman@wdc.com>
 <20240523125827.818-3-avri.altman@wdc.com> <Zk8-rwjFvgP714Mn@infradead.org>
 <DM6PR04MB65758584960580363D43AED4FCF42@DM6PR04MB6575.namprd04.prod.outlook.com>
 <Zk9Anwk1HEjUzSxc@infradead.org>
 <0a57d6bab739d6a10584f2baba115d00dfc9c94c.camel@mediatek.com>
Content-Language: en-US
From: Bart Van Assche <bvanassche@acm.org>
In-Reply-To: <0a57d6bab739d6a10584f2baba115d00dfc9c94c.camel@mediatek.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: quoted-printable

On 5/23/24 23:06, Peter Wang (=E7=8E=8B=E4=BF=A1=E5=8F=8B) 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);
>   }
>  =20
> +static void ufs_mtk_set_rtt(struct ufs_hba *hba)
> +{
> +       struct ufs_dev_info *dev_info =3D &hba->dev_info;
> +       u32 rtt =3D 0;
> +       u32 dev_rtt =3D 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 =3D=3D MTK_MAX_NUM_RTT)
> +               return;
> +
> +       rtt =3D 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 =3D {
>          .op_runtime_config   =3D ufs_mtk_op_runtime_config,
>          .mcq_config_resource =3D ufs_mtk_mcq_config_resource,
>          .config_esi          =3D ufs_mtk_config_esi,
> +       .set_rtt             =3D ufs_mtk_set_rtt,
>   };
>  =20
>   /**
> 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
>  =20
> +/* 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.