linux-kernel - Re: [PATCH v1 01/15] dt-binding: remoteproc: mediatek: Support dual-core SCP

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]

Date:   Thu, 2 Jun 2022 13:21:12 +0800
From:   Tinghan Shen <tinghan.shen@...iatek.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Mathieu Poirier <mathieu.poirier@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Lee Jones <lee.jones@...aro.org>,
        Benson Leung <bleung@...omium.org>,
        Guenter Roeck <groeck@...omium.org>,
        Daisuke Nojiri <dnojiri@...omium.org>,
        Sebastian Reichel <sebastian.reichel@...labora.com>,
        "Dustin L. Howett" <dustin@...ett.net>,
        Tzung-Bi Shih <tzungbi@...nel.org>,
        "Gustavo A. R. Silva" <gustavoars@...nel.org>,
        Prashant Malani <pmalani@...omium.org>,
        "Enric Balletbo i Serra" <enric.balletbo@...labora.com>,
        Brian Norris <briannorris@...omium.org>
CC:     <linux-remoteproc@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-mediatek@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>, <chrome-platform@...ts.linux.dev>,
        <Project_Global_Chrome_Upstream_Group@...iatek.com>,
        <weishunc@...gle.com>
Subject: Re: [PATCH v1 01/15] dt-binding: remoteproc: mediatek: Support
 dual-core SCP

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@...iatek.com>
> > ---
> >  .../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