linux-kernel - RE: [PATCH v3 5/9] media: chips-media: wave6: Add Wave6 core 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:
 <SL2P216MB1246E6FBFF73D2EDE1834F06FB07A@SL2P216MB1246.KORP216.PROD.OUTLOOK.COM>
Date: Mon, 1 Sep 2025 08:34:17 +0000
From: Nas Chung <nas.chung@...psnmedia.com>
To: Krzysztof Kozlowski <krzk@...nel.org>, "mchehab@...nel.org"
	<mchehab@...nel.org>, "hverkuil@...all.nl" <hverkuil@...all.nl>,
	"robh@...nel.org" <robh@...nel.org>, "krzk+dt@...nel.org"
	<krzk+dt@...nel.org>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
	"shawnguo@...nel.org" <shawnguo@...nel.org>, "s.hauer@...gutronix.de"
	<s.hauer@...gutronix.de>
CC: "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-imx@....com" <linux-imx@....com>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, jackson.lee
	<jackson.lee@...psnmedia.com>, lafley.kim <lafley.kim@...psnmedia.com>, Ming
 Qian <ming.qian@....nxp.com>
Subject: RE: [PATCH v3 5/9] media: chips-media: wave6: Add Wave6 core driver

Hi, Krzysztof.

>-----Original Message-----
>From: Krzysztof Kozlowski <krzk@...nel.org>
>Sent: Friday, August 29, 2025 11:03 PM
>To: Nas Chung <nas.chung@...psnmedia.com>; mchehab@...nel.org;
>hverkuil@...all.nl; robh@...nel.org; krzk+dt@...nel.org;
>conor+dt@...nel.org; shawnguo@...nel.org; s.hauer@...gutronix.de
>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 <jackson.lee@...psnmedia.com>;
>lafley.kim <lafley.kim@...psnmedia.com>; Ming Qian <ming.qian@....nxp.com>
>Subject: Re: [PATCH v3 5/9] media: chips-media: wave6: Add Wave6 core
>driver
>
>On 29/08/2025 10:46, Nas Chung wrote:
>> This adds the core driver for the Chips&Media Wave6 video codec IP.
>
>Please do not use "This commit/patch/change", but imperative mood. See
>longer explanation here:
>https://elixir.bootlin.com/linux/v6.16/source/Documentation/process/submitt
>ing-patches.rst#L94

Understood, I'll fix the commit message in v4.
Thanks for sharing it.

>
>>
>> The core region provides the encoding and decoding capabilities of
>> the VPU and depends on the control region for firmware and shared
>> resource management.
>>
>
>
>...
>
>> +
>> +static int wave6_vpu_core_probe(struct platform_device *pdev)
>> +{
>> +	struct vpu_core_device *core;
>> +	const struct wave6_vpu_core_resource *res;
>> +	int ret;
>> +	int irq;
>> +
>> +	res = device_get_match_data(&pdev->dev);
>> +	if (!res) {
>> +		dev_err(&pdev->dev, "missing resource\n");
>
>This is almost impossible condition. Not worth printing errors.

I'll remove it in v4.

>
>> +		return -EINVAL;
>> +	}
>> +
>> +	ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
>> +	if (ret < 0) {
>> +		dev_err(&pdev->dev, "dma_set_mask_and_coherent failed: %d\n",
>ret);
>> +		return ret;
>> +	}
>> +
>> +	core = devm_kzalloc(&pdev->dev, sizeof(*core), GFP_KERNEL);
>> +	if (!core)
>> +		return -ENOMEM;
>> +
>> +	ret = devm_mutex_init(&pdev->dev, &core->dev_lock);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = devm_mutex_init(&pdev->dev, &core->hw_lock);
>> +	if (ret)
>> +		return ret;
>> +
>> +	init_completion(&core->irq_done);
>> +	dev_set_drvdata(&pdev->dev, core);
>> +	core->dev = &pdev->dev;
>> +	core->res = res;
>> +
>> +	if (pdev->dev.parent->driver && pdev->dev.parent->driver->name &&
>> +	    !strcmp(pdev->dev.parent->driver->name,
>WAVE6_VPU_PLATFORM_DRIVER_NAME))
>> +		core->vpu = dev_get_drvdata(pdev->dev.parent);
>> +
>> +	core->reg_base = devm_platform_ioremap_resource(pdev, 0);
>> +	if (IS_ERR(core->reg_base))
>> +		return PTR_ERR(core->reg_base);
>> +
>> +	ret = devm_clk_bulk_get_all(&pdev->dev, &core->clks);
>> +	if (ret < 0) {
>> +		dev_warn(&pdev->dev, "unable to get clocks: %d\n", ret);
>
>You should handle deferred probe instead.

Okay, I'll address this in v4 using dev_err_probe().

>
>> +		ret = 0;
>> +	}
>> +	core->num_clks = ret;
>> +
>> +	ret = v4l2_device_register(&pdev->dev, &core->v4l2_dev);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "v4l2_device_register fail: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = wave6_vpu_init_m2m_dev(core);
>> +	if (ret)
>> +		goto err_v4l2_unregister;
>> +
>> +	irq = platform_get_irq(pdev, 0);
>> +	if (irq < 0) {
>> +		dev_err(&pdev->dev, "failed to get irq resource\n");
>
>Syntax is: dev_err_probe
>
>> +		ret = -ENXIO;
>
>Don't override error codes.

Understood, I'll clean up this part in v4 based on your feedback.

Thanks,
Nas.

>
>> +		goto err_m2m_dev_release;
>> +	}
>> +
>> +	ret = kfifo_alloc(&core->irq_status, 16 * sizeof(int), GFP_KERNEL);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "failed to allocate fifo\n");
>> +		goto err_m2m_dev_release;
>> +	}
>> +
>> +	ret = devm_request_threaded_irq(&pdev->dev, irq,
>> +					wave6_vpu_core_irq,
>> +					wave6_vpu_core_irq_thread,
>> +					0, "vpu_irq", core);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "fail to register interrupt
>handler: %d\n", ret);
>> +		goto err_kfifo_free;
>> +	}
>> +
>> +	core->temp_vbuf.size = ALIGN(W6_TEMPBUF_SIZE, 4096);
>> +	ret = wave6_vdi_alloc_dma(core->dev, &core->temp_vbuf);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "alloc temp of size %zu failed\n",
>> +			core->temp_vbuf.size);
>> +		goto err_kfifo_free;
>> +	}
>> +
>> +	core->debugfs = debugfs_lookup(WAVE6_VPU_DEBUGFS_DIR, NULL);
>> +	if (IS_ERR_OR_NULL(core->debugfs))
>> +		core->debugfs = debugfs_create_dir(WAVE6_VPU_DEBUGFS_DIR,
>NULL);
>> +
>> +	pm_runtime_enable(&pdev->dev);
>> +
>> +	if (core->res->codec_types & WAVE6_IS_DEC) {
>> +		ret = wave6_vpu_dec_register_device(core);
>> +		if (ret) {
>> +			dev_err(&pdev->dev, "wave6_vpu_dec_register_device
>fail: %d\n", ret);
>> +			goto err_temp_vbuf_free;
>> +		}
>> +	}
>> +	if (core->res->codec_types & WAVE6_IS_ENC) {
>> +		ret = wave6_vpu_enc_register_device(core);
>> +		if (ret) {
>> +			dev_err(&pdev->dev, "wave6_vpu_enc_register_device
>fail: %d\n", ret);
>> +			goto err_dec_unreg;
>> +		}
>> +	}
>Best regards,
>Krzysztof