* RE: [bug report] scsi: ufs: ufs-exynos: Add UFS host support for Exynos SoCs
2021-07-27 9:46 ` [bug report] scsi: ufs: ufs-exynos: Add UFS host support for Exynos SoCs Dan Carpenter
@ 2021-08-19 16:57 ` Alim Akhtar
0 siblings, 0 replies; 2+ messages in thread
From: Alim Akhtar @ 2021-08-19 16:57 UTC (permalink / raw)
To: 'Dan Carpenter'; +Cc: linux-scsi, 'Laurent Pinchart'
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
^ permalink raw reply [flat|nested] 2+ messages in thread