linux-kernel - Re: [PATCH v2 2/2] drivers: remoteproc: Add C906L controller for Sophgo CV1800B SoC

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: <1856ff20ba2bf450.56ed58d65d21d4ef.5a222480fa1019f2@Jude-Air.local>
Date: Wed, 30 Jul 2025 10:00:20 +0000
From: "Junhui Liu" <junhui.liu@...moral.tech>
To: "Krzysztof Kozlowski" <krzk@...nel.org>, 
	"Bjorn Andersson" <andersson@...nel.org>, 
	"Mathieu Poirier" <mathieu.poirier@...aro.org>, 
	"Rob Herring" <robh@...nel.org>, 
	"Krzysztof Kozlowski" <krzk+dt@...nel.org>, 
	"Conor Dooley" <conor+dt@...nel.org>, 
	"Chen Wang" <unicorn_wang@...look.com>, 
	"Inochi Amaoto" <inochiama@...il.com>, 
	"Philipp Zabel" <p.zabel@...gutronix.de>, 
	"Paul Walmsley" <paul.walmsley@...ive.com>, 
	"Palmer Dabbelt" <palmer@...belt.com>, "Albert Ou" <aou@...s.berkeley.edu>, 
	"Alexandre Ghiti" <alex@...ti.fr>
Cc: <linux-remoteproc@...r.kernel.org>, <devicetree@...r.kernel.org>, 
	<sophgo@...ts.linux.dev>, <linux-kernel@...r.kernel.org>, 
	<linux-riscv@...ts.infradead.org>
Subject: Re: [PATCH v2 2/2] drivers: remoteproc: Add C906L controller for Sophgo
	 CV1800B SoC

On 30/07/2025 11:35, Krzysztof Kozlowski wrote:
> On 30/07/2025 11:27, Junhui Liu wrote:
>> On 30/07/2025 08:46, Krzysztof Kozlowski wrote:
>>> On 28/07/2025 13:03, Junhui Liu wrote:
>>>> +
>>>> +static int cv1800b_c906l_mem_alloc(struct rproc *rproc,
>>>> +				   struct rproc_mem_entry *mem)
>>>> +{
>>>> +	void __iomem *va;
>>>> +
>>>> +	va = ioremap_wc(mem->dma, mem->len);
>>>> +	if (!va)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	/* Update memory entry va */
>>>> +	mem->va = (void *)va;
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static int cv1800b_c906l_mem_release(struct rproc *rproc,
>>>> +				     struct rproc_mem_entry *mem)
>>>> +{
>>>> +	iounmap((void __iomem *)mem->va);
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static int cv1800b_c906l_add_carveout(struct rproc *rproc)
>>>> +{
>>>> +	struct device *dev = rproc->dev.parent;
>>>> +	struct device_node *np = dev->of_node;
>>>> +	struct of_phandle_iterator it;
>>>> +	struct rproc_mem_entry *mem;
>>>> +	struct reserved_mem *rmem;
>>>> +	int i = 0;
>>>> +
>>>> +	/* Register associated reserved memory regions */
>>>> +	of_phandle_iterator_init(&it, np, "memory-region", NULL, 0);
>>>> +	while (of_phandle_iterator_next(&it) == 0) {
>>>> +		rmem = of_reserved_mem_lookup(it.node);
>>>> +		if (!rmem) {
>>>> +			of_node_put(it.node);
>>>> +			return -EINVAL;
>>>> +		}
>>>> +
>>>> +		if (!strcmp(it.node->name, "vdev0buffer")) {
>>>
>>> Why are you adding undocumented ABI? And so hidden, not even using
>>> standard OF API!
>>>
>>> How does this behaves when I change your DTS to call it
>>> "whateverbuffer"? Does it work? Obviously not.
>>>
>>> No, stop doing that.
>> 
>> Yes, you're right. I will consider introducing a "memory-region-names"
>> property in the bindings, instead of relying on the node labels directly.
> 
> 
> You don't need it. First, you use some old code as template, but you
> should look how or re-use Rob's code rewriting this completely.

Sorry, I didn't catch up with that patch. I will look into it and update
my implementation accordingly.

> 
> Second, list has strict order, so you know exactly where the vdev0
> buffer is. It cannot be on any other position of the list.

Thanks for the advice, I will use the order in the list to identify the
memory region.

> 
> This is why you define the ABI. Use then the ABI.
> 

Understood, I will reconsider this.

-- 
Best regards,
Junhui Liu