> -----Original Message----- > From: Dan Carpenter [mailto:dan.carpenter@xxxxxxxxxx] > Sent: Wednesday, March 9, 2022 6:44 PM > To: Ming Qian <ming.qian@xxxxxxx> > Cc: linux-media@xxxxxxxxxxxxxxx > Subject: [EXT] [bug report] media: amphion: add vpu core driver > > Caution: EXT Email > > 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; > Got it, I'll fix it > 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
