Re: [bug report] media: amphion: add vpu core driver

[Dan Carpenter <dan.carpenter@oracle.com>]

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