On 02/06/2022 07:21, Tinghan Shen wrote:
> Hi Krzysztof,
>
> On Wed, 2022-06-01 at 13:50 +0200, Krzysztof Kozlowski wrote:
>> On 01/06/2022 13:21, Tinghan Shen wrote:
>>> The SCP co-processor is a dual-core RISC-V MCU on MT8195.
>>>
>>> Add a new property to identify each core and helps to find drivers
>>> through device tree API to cooperate with each other, e.g. boot flow and
>>> watchdog timeout flow.
>>>
>>> Add a new compatile for the driver of SCP 2nd core.
>>>
>>> Signed-off-by: Tinghan Shen <tinghan.shen@xxxxxxxxxxxx>
>>> ---
>>> .../devicetree/bindings/remoteproc/mtk,scp.yaml | 12 ++++++++++++
>>> 1 file changed, 12 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
>>> b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
>>> index eec3b9c4c713..b181786d9575 100644
>>> --- a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
>>> +++ b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
>>> @@ -20,6 +20,7 @@ properties:
>>> - mediatek,mt8186-scp
>>> - mediatek,mt8192-scp
>>> - mediatek,mt8195-scp
>>> + - mediatek,mt8195-scp-dual
>>>
>>> reg:
>>> description:
>>> @@ -57,6 +58,16 @@ properties:
>>> memory-region:
>>> maxItems: 1
>>>
>>> + mediatek,scp-core:
>>> + $ref: /schemas/types.yaml#/definitions/uint32-array
>>> + description:
>>> + The property value is a list with 2 items, a core id and a phandle
>>
>> uint32, not phandle.
>>
>>> + to the sibling SCP node.
>>
>> Skip this. First part is obvious from the schema, second part should be
>> described via items.
>>
>> The core id represents the id of the dts node contains
>>> + this property. The valid values of core id are 0 and 1 for dual-core SCP.
>>> + The phandle of sibling SCP node is used to find the register settings,
>>> + trigger core dependent callback, and invoke rproc API.
>>
>> Entire description did not help me to understand what's this. So far it
>> looks like it is not a hardware property but some programming help, so
>> it does not look like properly described in bindings.
>>
>>> + maxItems: 1
>>
>> In description you said - two items.
>>
>> You need allOf:if:then disallowing this property for other variants.
>>
>>> +
>>> required:
>>> - compatible
>>> - reg
>>> @@ -115,6 +126,7 @@ examples:
>>> reg-names = "sram", "cfg", "l1tcm";
>>> clocks = <&infracfg CLK_INFRA_SCPSYS>;
>>> clock-names = "main";
>>> + mediatek,scp-core = <0 &scp_dual>;
>>
>> This looks like phandle, so wrong type.
>>>
>>> cros_ec {
>>> mediatek,rpmsg-name = "cros-ec-rpmsg";
>>
>
> Thanks for your feedback.
> After looking for a comparable uses case, I find out a different approach.
>
> mediatek,scp-core:
> $ref: "/schemas/types.yaml#/definitions/phandle-array"
> description:
> Enable the dual-core support in scp driver.
You describe desired functional behavior, not the hardware. What is the
property about? If you just want to indicate this is two-core processor,
then it could be:
mediatek,cores = <2>; /* number of cores */
However it seems you want to achieve here something different and as I
raised last time - it does not look like DT property.
Or maybe this is for first core and you want to indicate the sibling?
Something like that was mentioned in previous description.
> items:
> - items:
> - description: Assign a core id for current scp node.
> enum: [0, 1]
> - description:
> Phandle of another SCP node. This helps to find
> the scp driver of another core to trigger core
> dependent callback, invoke rproc subdevice API, etc.
Items should be rather reversed, as [0,1] being the argument to phandle
for a provider (see examples with syscon)...
Best regards,
Krzysztof
