RE: [bug report] scsi: ufs: ufs-exynos: Add UFS host support for Exynos SoCs

["Alim Akhtar" <alim.akhtar@samsung.com>]

Hi Dan
Sorry for the delay in responding; I had overlooked this.

> -----Original Message-----
> From: Dan Carpenter <dan.carpenter@oracle.com>
> Sent: 27 July 2021 15:16
> To: alim.akhtar@samsung.com
> Cc: linux-scsi@vger.kernel.org; Laurent Pinchart
> <laurent.pinchart@ideasonboard.com>
> Subject: [bug report] scsi: ufs: ufs-exynos: Add UFS host support for
Exynos
> SoCs
> 
> Hello Alim Akhtar,
> 
> The patch 55f4b1f73631: "scsi: ufs: ufs-exynos: Add UFS host support for
> Exynos SoCs" from May 28, 2020, leads to the following static checker
> warning:
> 
> 	drivers/scsi/ufs/ufs-exynos.c:286 exynos_ufs_get_clk_info()
> 	warn: wrong type for 'ufs->mclk_rate' (should be 'ulong')

> 
> 	drivers/scsi/ufs/ufs-exynos.c:287 exynos_ufs_get_clk_info()
> 	warn: wrong type for 'pclk_rate' (should be 'ulong')
> 
> drivers/scsi/ufs/ufs-exynos.c
>     258 static int exynos_ufs_get_clk_info(struct exynos_ufs *ufs)
>     259 {
>     260 	struct ufs_hba *hba = ufs->hba;
>     261 	struct list_head *head = &hba->clk_list_head;
>     262 	struct ufs_clk_info *clki;
>     263 	u32 pclk_rate;
>                 ^^^^^^^^^^^^^
> 
>     264 	u32 f_min, f_max;
>     265 	u8 div = 0;
>     266 	int ret = 0;
>     267
>     268 	if (list_empty(head))
>     269 		goto out;
>     270
>     271 	list_for_each_entry(clki, head, list) {
>     272 		if (!IS_ERR(clki->clk)) {
>     273 			if (!strcmp(clki->name, "core_clk"))
>     274 				ufs->clk_hci_core = clki->clk;
>     275 			else if (!strcmp(clki->name,
"sclk_unipro_main"))
>     276 				ufs->clk_unipro_main = clki->clk;
>     277 		}
>     278 	}
>     279
>     280 	if (!ufs->clk_hci_core || !ufs->clk_unipro_main) {
>     281 		dev_err(hba->dev, "failed to get clk info\n");
>     282 		ret = -EINVAL;
>     283 		goto out;
>     284 	}
>     285
> --> 286 	ufs->mclk_rate = clk_get_rate(ufs->clk_unipro_main);
> --> 287 	pclk_rate = clk_get_rate(ufs->clk_hci_core);
> 
> This a new Smatch warning which is not yet pushed.  The clk_get_rate()
> function returns unsigned long so I guess ufs->mclk_rate and pclk_rate
> should be changed from u32.  Not sure the runtime impact.
> 
Thanks for reporting this, I checked ufs functionality with this change, no
regression.
Will be posting a fix soon.

>     288 	f_min = ufs->pclk_avail_min;
>     289 	f_max = ufs->pclk_avail_max;
>     290
>     291 	if (ufs->opts & EXYNOS_UFS_OPT_HAS_APB_CLK_CTRL) {
>     292 		do {
>     293 			pclk_rate /= (div + 1);
>     294
>     295 			if (pclk_rate <= f_max)
>     296 				break;
>     297 			div++;
>     298 		} while (pclk_rate >= f_min);
>     299 	}
>     300
>     301 	if (unlikely(pclk_rate < f_min || pclk_rate > f_max)) {
>     302 		dev_err(hba->dev, "not available pclk range %d\n",
> pclk_rate);
>     303 		ret = -EINVAL;
>     304 		goto out;
>     305 	}
>     306
>     307 	ufs->pclk_rate = pclk_rate;
>     308 	ufs->pclk_div = div;
>     309
>     310 out:
>     311 	return ret;
>     312 }
> 
> regards,
> dan carpenter