Re: [bug report] scsi: ufs: core: mcq: Configure resource regions

[Asutosh Das <quic_asutoshd@quicinc.com>]

On Mon, Feb 27 2023 at 00:16 -0800, Dan Carpenter wrote:
>Hello Asutosh Das,
>
>The patch c263b4ef737e: "scsi: ufs: core: mcq: Configure resource
>regions" from Jan 13, 2023, leads to the following Smatch static
>checker warning:
>
>drivers/ufs/host/ufs-qcom.c:1455 ufs_qcom_mcq_config_resource() warn: passing zero to 'PTR_ERR'
>drivers/ufs/host/ufs-qcom.c:1469 ufs_qcom_mcq_config_resource() info: returning a literal zero is cleaner
>

Hello Dan
Thanks for reporting this. I will fix and push a change sometime this week.

-Asd

>drivers/ufs/host/ufs-qcom.c
>    1425 static int ufs_qcom_mcq_config_resource(struct ufs_hba *hba)
>    1426 {
>    1427         struct platform_device *pdev = to_platform_device(hba->dev);
>    1428         struct ufshcd_res_info *res;
>    1429         struct resource *res_mem, *res_mcq;
>    1430         int i, ret = 0;
>    1431
>    1432         memcpy(hba->res, ufs_res_info, sizeof(ufs_res_info));
>    1433
>    1434         for (i = 0; i < RES_MAX; i++) {
>    1435                 res = &hba->res[i];
>    1436                 res->resource = platform_get_resource_byname(pdev,
>    1437                                                              IORESOURCE_MEM,
>    1438                                                              res->name);
>    1439                 if (!res->resource) {
>    1440                         dev_info(hba->dev, "Resource %s not provided\n", res->name);
>    1441                         if (i == RES_UFS)
>    1442                                 return -ENOMEM;
>    1443                         continue;
>    1444                 } else if (i == RES_UFS) {
>    1445                         res_mem = res->resource;
>    1446                         res->base = hba->mmio_base;
>    1447                         continue;
>    1448                 }
>    1449
>    1450                 res->base = devm_ioremap_resource(hba->dev, res->resource);
>    1451                 if (IS_ERR(res->base)) {
>    1452                         dev_err(hba->dev, "Failed to map res %s, err=%d\n",
>    1453                                          res->name, (int)PTR_ERR(res->base));
>    1454                         res->base = NULL;
>                                 ^^^^^^^^^^^^^^^^
>
>--> 1455                         ret = PTR_ERR(res->base);
>                                 ^^^^^^^^^^^^^^^^^^^^^^^^
>Need to preserve the error code beforse setting this to NULL.
>
>    1456                         return ret;
>    1457                 }
>    1458         }
>    1459
>    1460         /* MCQ resource provided in DT */
>    1461         res = &hba->res[RES_MCQ];
>    1462         /* Bail if MCQ resource is provided */
>    1463         if (res->base)
>    1464                 goto out;
>    1465
>    1466         /* Explicitly allocate MCQ resource from ufs_mem */
>    1467         res_mcq = devm_kzalloc(hba->dev, sizeof(*res_mcq), GFP_KERNEL);
>    1468         if (!res_mcq)
>    1469                 return ret;
>
>Return -ENOMEM;
>
>    1470
>    1471         res_mcq->start = res_mem->start +
>    1472                          MCQ_SQATTR_OFFSET(hba->mcq_capabilities);
>    1473         res_mcq->end = res_mcq->start + hba->nr_hw_queues * MCQ_QCFG_SIZE - 1;
>    1474         res_mcq->flags = res_mem->flags;
>    1475         res_mcq->name = "mcq";
>    1476
>    1477         ret = insert_resource(&iomem_resource, res_mcq);
>    1478         if (ret) {
>    1479                 dev_err(hba->dev, "Failed to insert MCQ resource, err=%d\n",
>    1480                         ret);
>    1481                 goto insert_res_err;
>    1482         }
>    1483
>    1484         res->base = devm_ioremap_resource(hba->dev, res_mcq);
>    1485         if (IS_ERR(res->base)) {
>    1486                 dev_err(hba->dev, "MCQ registers mapping failed, err=%d\n",
>    1487                         (int)PTR_ERR(res->base));
>    1488                 ret = PTR_ERR(res->base);
>    1489                 goto ioremap_err;
>    1490         }
>    1491
>    1492 out:
>    1493         hba->mcq_base = res->base;
>    1494         return 0;
>    1495 ioremap_err:
>    1496         res->base = NULL;
>    1497         remove_resource(res_mcq);
>    1498 insert_res_err:
>    1499         devm_kfree(hba->dev, res_mcq);
>
>No need to call devm_kfree().  The whole point of devm_ is that it's
>automatic.
>
>    1500         return ret;
>    1501 }
>
>regards,
>dan carpenter