[bug report] dmaengine: qcom_hidma: simplify DT resource parsing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Rob Herring,

Commit 37fa4905d22a ("dmaengine: qcom_hidma: simplify DT resource
parsing") from Jan 4, 2018 (linux-next), leads to the following
Smatch static checker warning:

	drivers/dma/qcom/hidma_mgmt.c:408 hidma_mgmt_of_populate_channels()
	warn: both zero and negative are errors 'ret'

drivers/dma/qcom/hidma_mgmt.c
    348 static int __init hidma_mgmt_of_populate_channels(struct device_node *np)
    349 {
    350         struct platform_device *pdev_parent = of_find_device_by_node(np);
    351         struct platform_device_info pdevinfo;
    352         struct device_node *child;
    353         struct resource *res;
    354         int ret = 0;
    355 
    356         /* allocate a resource array */
    357         res = kcalloc(3, sizeof(*res), GFP_KERNEL);
    358         if (!res)
    359                 return -ENOMEM;
    360 
    361         for_each_available_child_of_node(np, child) {
    362                 struct platform_device *new_pdev;
    363 
    364                 ret = of_address_to_resource(child, 0, &res[0]);
    365                 if (!ret)
    366                         goto out;

This if statement seems reversed.  It will exit with success on the
first iteration through the loop.

    367 
    368                 ret = of_address_to_resource(child, 1, &res[1]);
    369                 if (!ret)
    370                         goto out;

Same.

    371 
    372                 ret = of_irq_to_resource(child, 0, &res[2]);
    373                 if (ret <= 0)
    374                         goto out;

This is actually what triggers the warning.  of_irq_to_resource()
returns a mix of negative error codes and zero on failure and positive
values on success.

    375 
    376                 memset(&pdevinfo, 0, sizeof(pdevinfo));
    377                 pdevinfo.fwnode = &child->fwnode;
    378                 pdevinfo.parent = pdev_parent ? &pdev_parent->dev : NULL;
    379                 pdevinfo.name = child->name;
    380                 pdevinfo.id = object_counter++;
    381                 pdevinfo.res = res;
    382                 pdevinfo.num_res = 3;
    383                 pdevinfo.data = NULL;
    384                 pdevinfo.size_data = 0;
    385                 pdevinfo.dma_mask = DMA_BIT_MASK(64);
    386                 new_pdev = platform_device_register_full(&pdevinfo);
    387                 if (IS_ERR(new_pdev)) {
    388                         ret = PTR_ERR(new_pdev);
    389                         goto out;
    390                 }
    391                 new_pdev->dev.of_node = child;
    392                 of_dma_configure(&new_pdev->dev, child, true);
    393                 /*
    394                  * It is assumed that calling of_msi_configure is safe on
    395                  * platforms with or without MSI support.
    396                  */
    397                 of_msi_configure(&new_pdev->dev, child);
    398         }
    399 
    400         kfree(res);
    401 
    402         return ret;

This should just be "return 0;" otherwise it's returning the positive
result from of_irq_to_resource().

    403 
    404 out:
    405         of_node_put(child);
    406         kfree(res);
    407 
--> 408         return ret;
    409 }

regards,
dan carpenter




[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux PCI]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux