Same thing in vpu_probe() as well.
drivers/media/platform/amphion/vpu_drv.c:122 vpu_probe() warn: pm_runtime_get_sync() also returns 1 on success
regards,
dan carpenter
On Wed, Mar 09, 2022 at 01:43:37PM +0300, Dan Carpenter wrote:
> Hello Ming Qian,
>
> The patch 9f599f351e86: "media: amphion: add vpu core driver" from
> Feb 24, 2022, leads to the following Smatch static checker warning:
>
> drivers/media/platform/amphion/vpu_core.c:654 vpu_core_probe()
> warn: pm_runtime_get_sync() also returns 1 on success
>
> drivers/media/platform/amphion/vpu_core.c
> 577 static int vpu_core_probe(struct platform_device *pdev)
> 578 {
> 579 struct device *dev = &pdev->dev;
> 580 struct vpu_core *core;
> 581 struct vpu_dev *vpu = dev_get_drvdata(dev->parent);
> 582 struct vpu_shared_addr *iface;
> 583 u32 iface_data_size;
> 584 int ret;
> 585
> 586 dev_dbg(dev, "probe\n");
> 587 if (!vpu)
> 588 return -EINVAL;
> 589 core = devm_kzalloc(dev, sizeof(*core), GFP_KERNEL);
> 590 if (!core)
> 591 return -ENOMEM;
> 592
> 593 core->pdev = pdev;
> 594 core->dev = dev;
> 595 platform_set_drvdata(pdev, core);
> 596 core->vpu = vpu;
> 597 INIT_LIST_HEAD(&core->instances);
> 598 mutex_init(&core->lock);
> 599 mutex_init(&core->cmd_lock);
> 600 init_completion(&core->cmp);
> 601 init_waitqueue_head(&core->ack_wq);
> 602 core->state = VPU_CORE_DEINIT;
> 603
> 604 core->res = of_device_get_match_data(dev);
> 605 if (!core->res)
> 606 return -ENODEV;
> 607
> 608 core->type = core->res->type;
> 609 core->id = of_alias_get_id(dev->of_node, "vpu_core");
> 610 if (core->id < 0) {
> 611 dev_err(dev, "can't get vpu core id\n");
> 612 return core->id;
> 613 }
> 614 dev_info(core->dev, "[%d] = %s\n", core->id, vpu_core_type_desc(core->type));
> 615 ret = vpu_core_parse_dt(core, dev->of_node);
> 616 if (ret)
> 617 return ret;
> 618
> 619 core->base = devm_platform_ioremap_resource(pdev, 0);
> 620 if (IS_ERR(core->base))
> 621 return PTR_ERR(core->base);
> 622
> 623 if (!vpu_iface_check_codec(core)) {
> 624 dev_err(core->dev, "is not supported\n");
> 625 return -EINVAL;
> 626 }
> 627
> 628 ret = vpu_mbox_init(core);
> 629 if (ret)
> 630 return ret;
> 631
> 632 iface = devm_kzalloc(dev, sizeof(*iface), GFP_KERNEL);
> 633 if (!iface)
> 634 return -ENOMEM;
> 635
> 636 iface_data_size = vpu_iface_get_data_size(core);
> 637 if (iface_data_size) {
> 638 iface->priv = devm_kzalloc(dev, iface_data_size, GFP_KERNEL);
> 639 if (!iface->priv)
> 640 return -ENOMEM;
> 641 }
> 642
> 643 ret = vpu_iface_init(core, iface, &core->rpc, core->fw.phys);
> 644 if (ret) {
> 645 dev_err(core->dev, "init iface fail, ret = %d\n", ret);
> 646 return ret;
> 647 }
> 648
> 649 vpu_iface_config_system(core, vpu->res->mreg_base, vpu->base);
> 650 vpu_iface_set_log_buf(core, &core->log);
> 651
> 652 pm_runtime_enable(dev);
> 653 ret = pm_runtime_get_sync(dev);
> --> 654 if (ret) {
> ^^^
> This isn't right.
>
> 655 pm_runtime_put_noidle(dev);
> 656 pm_runtime_set_suspended(dev);
> 657 goto err_runtime_disable;
> 658 }
>
> The documentation for pm_runtime_get_sync() suggests using
> pm_runtime_resume_and_get() instead. I think you can just do
>
> ret = pm_runtime_resume_and_get(dev);
> if (ret)
> goto err_runtime_disable;
>
> 659
> 660 ret = vpu_core_register(dev->parent, core);
> 661 if (ret)
> 662 goto err_core_register;
> 663 core->parent = dev->parent;
> 664
> 665 pm_runtime_put_sync(dev);
> 666 vpu_core_create_dbgfs_file(core);
> 667
> 668 return 0;
> 669
> 670 err_core_register:
> 671 pm_runtime_put_sync(dev);
> 672 err_runtime_disable:
> 673 pm_runtime_disable(dev);
> 674
> 675 return ret;
> 676 }
>
> regards,
> dan carpenter
