linux-kernel - Re: [PATCH 4/8] media: chips-media: wave6: Add Wave6 codec driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7373c956-216a-43a6-939c-4a9be5564c57@kernel.org>
Date: Mon, 10 Feb 2025 18:40:44 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Nas Chung <nas.chung@...psnmedia.com>, mchehab@...nel.org,
 hverkuil@...all.nl, sebastian.fricke@...labora.com, robh@...nel.org,
 krzk+dt@...nel.org, conor+dt@...nel.org
Cc: linux-media@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-imx@....com,
 linux-arm-kernel@...ts.infradead.org, jackson.lee@...psnmedia.com,
 lafley.kim@...psnmedia.com
Subject: Re: [PATCH 4/8] media: chips-media: wave6: Add Wave6 codec driver

On 10/02/2025 10:07, Nas Chung wrote:
> +
> +	dev->debugfs = debugfs_lookup(WAVE6_VPU_DEBUGFS_DIR, NULL);
> +	if (IS_ERR_OR_NULL(dev->debugfs))
> +		dev->debugfs = debugfs_create_dir(WAVE6_VPU_DEBUGFS_DIR, NULL);
> +
> +	pm_runtime_enable(&pdev->dev);
> +
> +	if (dev->res->codec_types & WAVE6_IS_DEC) {
> +		ret = wave6_vpu_dec_register_device(dev);
> +		if (ret) {
> +			dev_err(&pdev->dev, "wave6_vpu_dec_register_device fail: %d\n", ret);
> +			goto err_temp_vbuf_free;
> +		}
> +	}
> +	if (dev->res->codec_types & WAVE6_IS_ENC) {
> +		ret = wave6_vpu_enc_register_device(dev);
> +		if (ret) {
> +			dev_err(&pdev->dev, "wave6_vpu_enc_register_device fail: %d\n", ret);
> +			goto err_dec_unreg;
> +		}
> +	}
> +
> +	if (dev->ctrl && wave6_vpu_ctrl_support_follower(dev->ctrl)) {
> +		wave6_vpu_activate(dev);
> +		ret = pm_runtime_resume_and_get(dev->dev);
> +		if (ret)
> +			goto err_enc_unreg;
> +	}
> +
> +	dev_dbg(&pdev->dev, "Added wave6 driver with caps %s %s\n",
> +		dev->res->codec_types & WAVE6_IS_ENC ? "'ENCODE'" : "",
> +		dev->res->codec_types & WAVE6_IS_DEC ? "'DECODE'" : "");
> +
> +	return 0;
> +
> +err_enc_unreg:
> +	if (dev->res->codec_types & WAVE6_IS_ENC)
> +		wave6_vpu_enc_unregister_device(dev);
> +err_dec_unreg:
> +	if (dev->res->codec_types & WAVE6_IS_DEC)
> +		wave6_vpu_dec_unregister_device(dev);
> +err_temp_vbuf_free:
> +	wave6_free_dma(&dev->temp_vbuf);
> +err_kfifo_free:
> +	kfifo_free(&dev->irq_status);
> +err_m2m_dev_release:
> +	wave6_vpu_release_m2m_dev(dev);
> +err_v4l2_unregister:
> +	v4l2_device_unregister(&dev->v4l2_dev);
> +
> +	return ret;
> +}
> +
> +static void wave6_vpu_remove(struct platform_device *pdev)
> +{
> +	struct vpu_device *dev = dev_get_drvdata(&pdev->dev);
> +
> +	if (dev->ctrl && wave6_vpu_ctrl_support_follower(dev->ctrl)) {
> +		if (!pm_runtime_suspended(&pdev->dev))
> +			pm_runtime_put_sync(&pdev->dev);
> +	}
> +	pm_runtime_disable(&pdev->dev);
> +
> +	wave6_vpu_enc_unregister_device(dev);
> +	wave6_vpu_dec_unregister_device(dev);
> +	wave6_free_dma(&dev->temp_vbuf);
> +	kfifo_free(&dev->irq_status);
> +	wave6_vpu_release_m2m_dev(dev);
> +	v4l2_device_unregister(&dev->v4l2_dev);
> +}
> +
> +#ifdef CONFIG_PM
> +static int wave6_vpu_runtime_suspend(struct device *dev)
> +{
> +	struct vpu_device *vpu_dev = dev_get_drvdata(dev);
> +
> +	if (!vpu_dev)
> +		return -ENODEV;
> +
> +	dprintk(dev, "runtime suspend\n");

Drop

> +	if (vpu_dev->ctrl && vpu_dev->active)
> +		wave6_vpu_ctrl_put_sync(vpu_dev->ctrl, &vpu_dev->entity);
> +	if (vpu_dev->num_clks)
> +		clk_bulk_disable_unprepare(vpu_dev->num_clks, vpu_dev->clks);
> +
> +	return 0;
> +}
> +
> +static int wave6_vpu_runtime_resume(struct device *dev)
> +{
> +	struct vpu_device *vpu_dev = dev_get_drvdata(dev);
> +	int ret = 0;
> +
> +	if (!vpu_dev)
> +		return -ENODEV;
> +
> +	dprintk(dev, "runtime resume\n");

Drop

> +	if (vpu_dev->num_clks) {
> +		ret = clk_bulk_prepare_enable(vpu_dev->num_clks, vpu_dev->clks);
> +		if (ret) {
> +			dev_err(dev, "failed to enable clocks: %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	if (vpu_dev->ctrl && vpu_dev->active) {
> +		ret = wave6_vpu_ctrl_resume_and_get(vpu_dev->ctrl, &vpu_dev->entity);
> +		if (ret && vpu_dev->num_clks)
> +			clk_bulk_disable_unprepare(vpu_dev->num_clks, vpu_dev->clks);
> +	} else {
> +		wave6_vpu_check_state(vpu_dev);
> +	}
> +
> +	return ret;
> +}
> +#endif
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int wave6_vpu_suspend(struct device *dev)
> +{
> +	int ret;
> +
> +	dprintk(dev, "suspend\n");

Drop. Don't re-implement existing tracing.

> +	wave6_vpu_pause(dev, 0);
> +
> +	ret = pm_runtime_force_suspend(dev);
> +	if (ret)
> +		wave6_vpu_pause(dev, 1);
> +
> +	return ret;
> +}
> +
> +static int wave6_vpu_resume(struct device *dev)
> +{
> +	int ret;
> +
> +	dprintk(dev, "resume\n");

Drop, useless.

> +	ret = pm_runtime_force_resume(dev);
> +	if (ret)
> +		return ret;
> +
> +	wave6_vpu_pause(dev, 1);
> +	return 0;
> +}
> +#endif
> +static const struct dev_pm_ops wave6_vpu_pm_ops = {
> +	SET_RUNTIME_PM_OPS(wave6_vpu_runtime_suspend, wave6_vpu_runtime_resume, NULL)
> +	SET_SYSTEM_SLEEP_PM_OPS(wave6_vpu_suspend, wave6_vpu_resume)
> +};
> +
> +static const struct of_device_id wave6_dt_ids[] = {
> +	{ .compatible = "nxp,imx95-wave633c", .data = &wave633c_data },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, wave6_dt_ids);
> +
> +static struct platform_driver wave6_vpu_driver = {
> +	.driver = {
> +		.name = VPU_PLATFORM_DEVICE_NAME,
> +		.of_match_table = of_match_ptr(wave6_dt_ids),


Drop of_match_ptr, you have here warnings.


Best regards,
Krzysztof