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.
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.
>
> Best regards,
> Krzysztof
