linux-kernel - Re: [PATCH v2 1/2] dt-bindings: remoteproc: Add C906L rproc 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: <185679960f220550.906528684e28f712.ba6c1f3fdd9df549@Mac>
Date: Mon, 28 Jul 2025 17:13:10 +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 1/2] dt-bindings: remoteproc: Add C906L rproc for Sophgo
	 CV1800B SoC

On 28/07/2025 15:11, Krzysztof Kozlowski wrote:
> On 28/07/2025 13:03, Junhui Liu wrote:
>> +
>> +  firmware-name:
>> +    maxItems: 1
>> +
>> +  mbox-names:
>> +    items:
>> +      - const: tx
>> +      - const: rx
>> +
>> +  mboxes:
> 
> First go mboxes, then mboxes-names. ALWAYS.

Thanks, I will fix the order in the next version.

> 
>> +    description:
>> +      This property is required only if the rpmsg/virtio functionality is used.
>> +      (see mailbox/sophgo,cv1800b-mailbox.yaml)
>> +    items:
>> +      - description: mailbox channel to send data to C906L
>> +      - description: mailbox channel to receive data from C906L
>> +
>> +  memory-region:
>> +    description:
>> +      List of phandles to reserved memory regions used by the remote processor.
>> +      The first region is required and provides the firmware region for the
>> +      remote processor. The following regions (vdev buffer, vrings) are optional
>> +      and are only required if rpmsg/virtio functionality is used.
>> +    minItems: 1
> 
> Why isn't this constrained?

Do you mean a maxItems should be added here?

> 
>> +    items:
>> +      - description: firmware region
>> +      - description: vdev buffer
>> +      - description: vring0
>> +      - description: vring1
>> +    additionalItems: true
> 
> No, drop. This needs to be constrained.

My intention is that RPMsg/OpenAMP is not the only use case for
remoteproc. There are scenarios where the remoteproc is just used for
booting the remote processor without any communication with Linux. In
such case, only the firmware region is needed, and the other regions may
not be necessary.

Additionally, the remote processor might reserve extra memory for custom
buffers or other firmware-specific purposes beyond virtio/rpmsg.
Therefore, I think only the firmware region should be required and
constrained, while allowing flexibility for additional/custom memory
regions as needed.

> 
> 
>> +
>> +  resets:
>> +    maxItems: 1
>> +
>> +  sophgo,syscon:
>> +    $ref: /schemas/types.yaml#/definitions/phandle
>> +    description:
>> +      A phandle to the SEC_SYS region, used for configuration of the remote
>> +      processor.
>> +
>> +required:
>> +  - compatible
>> +  - firmware-name
>> +  - memory-region
>> +  - resets
>> +  - sophgo,syscon
> 
> Why mboxes are not required?

The reason is similar to "memory-region" above. The mboxes property is
only needed when RPMsg/virtio communication is used. For some use cases,
the remote processor does not need to communicate with Linux at all, so
the mailbox is not required. Would it be necessary to require the mboxes
property even in scenarios where mailbox communication is not involved?

> 
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    c906l-rproc {
> 
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

Thanks for the information, I will change the node name to a generic
"remoteproc" in the example. (Although it's not in the list, I think
it's generic enough)

> 
> 
> Best regards,
> Krzysztof

-- 
Best regards,
Junhui Liu