Re: [PATCH v2 1/2] dt-bindings: remoteproc: Add C906L rproc for Sophgo CV1800B SoC

From: Krzysztof Kozlowski <krzk@kernel.org>
To: Junhui Liu <junhui.liu@pigmoral.tech>,
	Bjorn Andersson <andersson@kernel.org>,
	Mathieu Poirier <mathieu.poirier@linaro.org>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Chen Wang <unicorn_wang@outlook.com>,
	Inochi Amaoto <inochiama@gmail.com>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Alexandre Ghiti <alex@ghiti.fr>
Cc: linux-remoteproc@vger.kernel.org, devicetree@vger.kernel.org,
	sophgo@lists.linux.dev, linux-kernel@vger.kernel.org,
	linux-riscv@lists.infradead.org
Subject: Re: [PATCH v2 1/2] dt-bindings: remoteproc: Add C906L rproc for Sophgo CV1800B SoC
Date: Wed, 30 Jul 2025 08:47:40 +0200	[thread overview]
Message-ID: <4137240e-a5c4-427b-900c-ae062aa9a9c8@kernel.org> (raw)
In-Reply-To: <1856e9e7bfdcb6a8.1f73e6be0161d31f.3ad0845628cf3b62@Jude-Air.local>

On 30/07/2025 05:31, Junhui Liu wrote:
> On 29/07/2025 08:27, Krzysztof Kozlowski wrote:
>> On 28/07/2025 19:13, Junhui Liu wrote:
>>>>
>>>>> +    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
>>
>> We don't allow such syntax, that's not negotiable. Why 322 redundant
>> memory regions are fine now?
>>
>>> 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.
>>
>> So how does this work exactly without the rest? Remote processor boots
>> and works fine? How do you communicate with it?
>>
>> Please describe exactly the usecase.
> 
> Thank you for your clarification.
> 
> The C906L remoteproc can run at two use cases:
> 1. Standalone mode: Only the firmware region is used. In this case, the
>    remoteproc driver loads the firmware into the firmware region and
>    boots the C906L. The C906L runs independently, without communication
>    with Linux, and the mailbox is not used.
> 2. OpenAMP/RPMsg mode: The firmware region, vdev buffer, and vrings are
>    used. In this scenario, the C906L runs firmware with OpenAMP support
>    and communicates with Linux via the virtio memory regions and mailbox.
> 
> To summarize:
> - Required: firmware region
> - Optional: vdev buffer, vrings, mailbox

How does your driver behave in (1)? Does it work?

Best regards,
Krzysztof