Message-ID: <1856fbcb55769ae0.5ecf256b8267239e.f51806827c8576a5@Jude-Air.local>
Date: Wed, 30 Jul 2025 08:59:15 +0000
From: "Junhui Liu" <junhui.liu@...moral.tech>
To: "Inochi Amaoto" <inochiama@...il.com>, 
	"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>, 
	"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 30/07/2025 14:05, Inochi Amaoto wrote:
> On Wed, Jul 30, 2025 at 03:57:09AM +0000, Junhui Liu wrote:
>> On 29/07/2025 16:31, Inochi Amaoto wrote:
>> > On Mon, Jul 28, 2025 at 07:03:23PM +0800, Junhui Liu wrote:
>> >> Add C906L remote processor for CV1800B SoC, which is an asymmetric
>> >> processor typically running RTOS.
>> >> 
>> >> Signed-off-by: Junhui Liu <junhui.liu@...moral.tech>
>> >> ---
>> >>  .../bindings/remoteproc/sophgo,cv1800b-c906l.yaml  | 79 ++++++++++++++++++++++
>> >>  1 file changed, 79 insertions(+)
>> >> 
>> >> diff --git a/Documentation/devicetree/bindings/remoteproc/sophgo,cv1800b-c906l.yaml b/Documentation/devicetree/bindings/remoteproc/sophgo,cv1800b-c906l.yaml
>> >> new file mode 100644
>> >> index 0000000000000000000000000000000000000000..2061c2fd6ba343c09b1a91700ea4a695d2b57f81
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/remoteproc/sophgo,cv1800b-c906l.yaml
>> >> @@ -0,0 +1,79 @@
>> >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> >> +%YAML 1.2
>> >> +---
>> >> +$id: http://devicetree.org/schemas/remoteproc/sophgo,cv1800b-c906l.yaml#
>> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> >> +
>> >> +title: Sophgo C906L remote processor controller for CV1800B SoC
>> >> +
>> >> +maintainers:
>> >> +  - Junhui Liu <junhui.liu@...moral.tech>
>> >> +
>> >> +description:
>> >> +  Document the bindings for the C906L remoteproc component that loads and boots
>> >> +  firmwares on the CV1800B SoC.
>> >> +
>> >> +properties:
>> >> +  compatible:
>> >> +    const: sophgo,cv1800b-c906l
>> >> +
>> >> +  firmware-name:
>> >> +    maxItems: 1
>> >> +
>> >> +  mbox-names:
>> >> +    items:
>> >> +      - const: tx
>> >> +      - const: rx
>> >> +
>> >> +  mboxes:
>> >> +    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
>> >> +    items:
>> >> +      - description: firmware region
>> >> +      - description: vdev buffer
>> >> +      - description: vring0
>> >> +      - description: vring1
>> >> +    additionalItems: true
>> >> +
>> > 
>> > Why not allocating these region dynamicly? I do not think firware is
>> > always avaible before staring. Allowing dynamic firmware give us max
>> > flexiblity.
>> 
>> I'm afraid it's not easy to do this.
>> 
>> For firmware region, the RTOS firmware usually needs a physical address
>> to link to, and I have researched and tested two RTOS (RT-Thread and
>> Zephyr) on the C906L, both of them do not support position-independent
>> execution or runtime relocation. Therefore, a reserved memory region is
>> needed to provide a fixed physical address for the RTOS firmware.
> 
> I think it is simple and possible to add PIE support for these RTOS. As
> the memory of CV18XX is limited, I do not want to see some reserved
> regions. This may hurt users who do not need this.

Thank you for sharing your concern about the limited memory.

However, I think I have to wait until some RTOS supports PIE before I
can continue to advance this patch series. At least I haven't found any
guide on compiling RTOS firmware with PIE support for the two RTOSs
(RT-Thread and Zephyr) I'm currently testing on the C906L.

Besides, I have searched the existing remoteproc drivers in the kernel,
and haven't found any driver using dynamic memory allocation for the
firmware region. It may take some time to implement this feature if we
really need it on CV18XX SoCs.

> 
>> (In fact, there is already a reserved memory region for the C906L in
>> cv1800b-milkv-duo.dts)
> 
> This is just preserved for vendor zsbl and I have a plan to remove it.
> Always let linux take care of all memory. It is good to support all
> firmware implementation for CV18XX.

Got it.

> 
> I think it is always good to use remoteproc like this:
> https://www.kernel.org/doc/html/latest/staging/remoteproc.html
> 
>> 
>> For virtio-related regions, the RTOS firmware also needs to know the
>> shared memory regions for communications at compile time.
>> 
> 
> I think you should investigate this and check if there is something you
> missed. I haven't see any reserved region in remoteproc binding mentions
> virtio.

Currently, in Zephyr, the only boards with OpenAMP sample support are
the i.MX and STM32MP series [1]. Both of them define reserved memory
regions for virtio and vrings in their respective Linux kernel device
trees [2][3]. These are the only available reference targets I have at
the moment.

Furthermore, searching for the keyword "vring" in the remoteproc
bindings yields many results, which I believe mostly pertain to reserved
memory regions for rpmsg/virtio.

$grep "vring" -r Documentation/devicetree/bindings/remoteproc | wc -l
24

So, at present, all my references use reserved memory for firmware
regions (unless there is specific memory for the processor in hardware)
and for virtio-related regions. Do you think there is anything I might
have missed, or should some new feature be implemented?

[1] https://github.com/zephyrproject-rtos/zephyr/tree/main/samples/subsys/ipc/openamp_rsc_table/boards
[2] https://github.com/torvalds/linux/blob/v6.16/arch/arm/boot/dts/st/stm32mp15xx-dkx.dtsi#L33-L49
[3] https://github.com/torvalds/linux/blob/v6.16/arch/arm64/boot/dts/freescale/imx8dxl-evk.dts#L68-L97

-- 
Best regards,
Junhui Liu

Powered by blists - more mailing lists