Message ID | 20230327140342.222168-2-cristian.marussi@arm.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp1546660vqo; Mon, 27 Mar 2023 07:17:23 -0700 (PDT) X-Google-Smtp-Source: AKy350YQi9skAhTmdJ/ZBP6yVA2KVO9Tv0QwZf1+afMIYN4hEVoIerB5gJlAGsEe9pQPSt4JWPt5 X-Received: by 2002:a62:6385:0:b0:627:e175:2b5f with SMTP id x127-20020a626385000000b00627e1752b5fmr10594792pfb.3.1679926643119; Mon, 27 Mar 2023 07:17:23 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1679926643; cv=none; d=google.com; s=arc-20160816; b=fDTmoJdwFzEoRCrgKNS1OTFLAOzM5o/TkMsKbsqOLwzL2OB/oz1BF0kP/9OmYntSm8 +zN4tzPfb+BLhQF7aHOYTcMdDc/D43q7GUHr/Fi+wnaZ+qFIy4z0gzFCggJ2R05U5LpE /nEtwApFlGaNsIsIXYzF2retY5YOou6akyRgYnb/+TRdv4CtEfDWbAOYNHcg82919x94 2se5UpmvZd0nfWNe7BtsmPKzoEyppA7Wn3b3pGckM5Dbs16tne9npPE0SVvgj+oAZSEl rBhcgsyMW56XzEyHfuqcZqr/NG+n1I9Y2wsdebOQgM5TIC3FTh6Ri0ch5g4FcV/hAtKV p8uA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from; bh=ieSJNOH3wGsC9Zkd+ubugjlbixShaPYBe5Tqpt9E7uE=; b=NFX3WPQxp6qd/v0Ka36sgHBSFhqXx58DeXh9dEtN19CNLde2cbvEXxMD7r67we0IHE yyPjQJAWqb8z/kWWXkLIctztOW5fo6G3J5GWXs9yEfw2VsCalEFsejoOCHEvc3BLiBAJ vRvA0MJM3AD2Op7B6yAiGbY3B4mZvJFwcvTa219syiTMP0SnJz0Y1ARmf+/1cpL2xeUv Dirasm9vdc48vRBYZb6YLEH7Vjg7C83skkDTjeCx6awjSkGlbvjwcq3yeSS4stumO4Gj fIWrGgrAF7oKc/xia0NAH6C/bFskAhW5W9oB+uIuhvfKMaST0iddSUPCKZ2qe3Ei9xfW wr3A== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id p7-20020a056a000a0700b005a8b856ad4csi27817995pfh.205.2023.03.27.07.17.09; Mon, 27 Mar 2023 07:17:23 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229525AbjC0OEr (ORCPT <rfc822;makky5685@gmail.com> + 99 others); Mon, 27 Mar 2023 10:04:47 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36606 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229619AbjC0OEn (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 27 Mar 2023 10:04:43 -0400 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id ABFCE1B7; Mon, 27 Mar 2023 07:04:24 -0700 (PDT) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 9A62E1042; Mon, 27 Mar 2023 07:05:08 -0700 (PDT) Received: from e120937-lin.. (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 087A93F6C4; Mon, 27 Mar 2023 07:04:22 -0700 (PDT) From: Cristian Marussi <cristian.marussi@arm.com> To: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org Cc: sudeep.holla@arm.com, vincent.guittot@linaro.org, souvik.chakravarty@arm.com, nicola.mazzucato@arm.com, cristian.marussi@arm.com, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org Subject: [PATCH 1/2] dt-bindings: firmware: arm,scmi: Support mailboxes unidirectional channels Date: Mon, 27 Mar 2023 15:03:41 +0100 Message-Id: <20230327140342.222168-2-cristian.marussi@arm.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230327140342.222168-1-cristian.marussi@arm.com> References: <20230327140342.222168-1-cristian.marussi@arm.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.3 required=5.0 tests=RCVD_IN_DNSWL_MED, SPF_HELO_NONE,SPF_NONE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1761530759691433718?= X-GMAIL-MSGID: =?utf-8?q?1761530759691433718?= |
Series |
Add SCMI support for mailbox unidirectional channels
|
|
Commit Message
Cristian Marussi
March 27, 2023, 2:03 p.m. UTC
SCMI defines two kinds of communication channels between the agent and the
platform: one bidirectional 'a2p' channel used by the agent to send SCMI
commands and synchronously receive the related replies, and an optional
'p2a' unidirectional channel used to asynchronously receive delayed
responses and notifications emitted from the platform.
When configuring an SCMI transport based on mailboxes, the current binding
supports only mailboxes providing bidirectional channels: in such a case
one mailbox channel can be easily assigned to each SCMI channel as above
described.
In case, instead, to have to deal with mailboxes providing only distinct
unidirectional channels, it becomes necessary to extend the binding in
order to be able to bind 2 distinct unidirectional mailbox channels to the
same SCMI 'a2p' channel.
Bidirectional and unidirectional channels support for the SCMI mailbox
transport can coexist by carefully considering the effective combination
of defined 'mboxes' and 'shmem' descriptors.
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
.../bindings/firmware/arm,scmi.yaml | 42 +++++++++++++++++--
1 file changed, 38 insertions(+), 4 deletions(-)
Comments
On 27/03/2023 16:03, Cristian Marussi wrote: > SCMI defines two kinds of communication channels between the agent and the > platform: one bidirectional 'a2p' channel used by the agent to send SCMI > commands and synchronously receive the related replies, and an optional > 'p2a' unidirectional channel used to asynchronously receive delayed > responses and notifications emitted from the platform. > > When configuring an SCMI transport based on mailboxes, the current binding > supports only mailboxes providing bidirectional channels: in such a case > one mailbox channel can be easily assigned to each SCMI channel as above > described. > > In case, instead, to have to deal with mailboxes providing only distinct > unidirectional channels, it becomes necessary to extend the binding in > order to be able to bind 2 distinct unidirectional mailbox channels to the > same SCMI 'a2p' channel. > > Bidirectional and unidirectional channels support for the SCMI mailbox > transport can coexist by carefully considering the effective combination > of defined 'mboxes' and 'shmem' descriptors. > > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> > Cc: devicetree@vger.kernel.org Please drop the autogenerated scripts/get_maintainer.pl CC-entries from commit msg. There is no single need to store automated output of get_maintainers.pl in the git log. It can be easily re-created at any given time, thus its presence in the git history is redundant and obfuscates the log. If you need it for your own patch management purposes, keep it under ---. > Signed-off-by: Cristian Marussi <cristian.marussi@arm.com> > --- > .../bindings/firmware/arm,scmi.yaml | 42 +++++++++++++++++-- > 1 file changed, 38 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml > index 2f7c51c75e85..9a7dc30e386f 100644 > --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml > +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml > @@ -63,10 +63,24 @@ properties: > mboxes: > description: > List of phandle and mailbox channel specifiers. It should contain > - exactly one or two mailboxes, one for transmitting messages("tx") > - and another optional for receiving the notifications("rx") if supported. > + exactly one, two or three mailboxes; the first one or two for transmitting > + messages ("tx") and another optional ("rx") for receiving notifications > + and delayed responses, if supported by the platform. > + The number of mailboxes needed for transmitting messages depends on the > + type of channels exposed by the specific underlying mailbox controller; > + one single channel descriptor is enough if such channel is bidirectional, > + while two channel descriptors are needed to represent the SCMI ("tx") > + channel if the underlying mailbox channels are of unidirectional type. > + The effective combination in numbers of mboxes and shmem descriptors let > + the SCMI subsystem determine unambiguosly which type of SCMI channels are > + made available by the underlying mailbox controller and how to use them. > + 1 mbox / 1 shmem => SCMI TX over 1 mailbox bidirectional channel > + 2 mbox / 2 shmem => SCMI TX and RX over 2 mailbox bidirectional channels > + 2 mbox / 1 shmem => SCMI TX over 2 mailbox unidirectional channels > + 3 mbox / 2 shmem => SCMI TX and RX over 3 mailbox unidirectional channels > + Any other combination of mboxes and shmem is invalid. > minItems: 1 > - maxItems: 2 > + maxItems: 3 Missing update to mbox-names. > > shmem: > description: > @@ -234,7 +248,7 @@ $defs: > > mboxes: > minItems: 1 > - maxItems: 2 > + maxItems: 3 The same. How is it supposed to work? tx rx and that's it? > > shmem: > minItems: 1 > @@ -393,6 +407,26 @@ examples: > }; > }; > > + - | > + firmware { > + scmi { > + compatible = "arm,scmi"; > + mboxes = <&mhu_U_tx 0 0>, <&mhu_U_rx 0 0>; > + shmem = <&cpu_scp_lpri0>; > + > + #address-cells = <1>; > + #size-cells = <0>; I don't think adding one more example with difference in only one piece is needed here. Best regards, Krzysztof
On Mon, Mar 27, 2023 at 04:42:12PM +0200, Krzysztof Kozlowski wrote: > On 27/03/2023 16:03, Cristian Marussi wrote: > > SCMI defines two kinds of communication channels between the agent and the > > platform: one bidirectional 'a2p' channel used by the agent to send SCMI > > commands and synchronously receive the related replies, and an optional > > 'p2a' unidirectional channel used to asynchronously receive delayed > > responses and notifications emitted from the platform. > > > > When configuring an SCMI transport based on mailboxes, the current binding > > supports only mailboxes providing bidirectional channels: in such a case > > one mailbox channel can be easily assigned to each SCMI channel as above > > described. > > > > In case, instead, to have to deal with mailboxes providing only distinct > > unidirectional channels, it becomes necessary to extend the binding in > > order to be able to bind 2 distinct unidirectional mailbox channels to the > > same SCMI 'a2p' channel. > > > > Bidirectional and unidirectional channels support for the SCMI mailbox > > transport can coexist by carefully considering the effective combination > > of defined 'mboxes' and 'shmem' descriptors. > > > > Cc: Rob Herring <robh+dt@kernel.org> > > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> > > Cc: devicetree@vger.kernel.org > Hi Krzysztof, thanks for having a look. > Please drop the autogenerated scripts/get_maintainer.pl CC-entries from > commit msg. There is no single need to store automated output of > get_maintainers.pl in the git log. It can be easily re-created at any > given time, thus its presence in the git history is redundant and > obfuscates the log. > > If you need it for your own patch management purposes, keep it under ---. > Ok. I use to add those Cc to trigger git-sendmail to add proper CCs but in this case indeed I copied you on all the series anyway. I'll drop it. > > > Signed-off-by: Cristian Marussi <cristian.marussi@arm.com> > > --- > > .../bindings/firmware/arm,scmi.yaml | 42 +++++++++++++++++-- > > 1 file changed, 38 insertions(+), 4 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml > > index 2f7c51c75e85..9a7dc30e386f 100644 > > --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml > > +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml > > @@ -63,10 +63,24 @@ properties: > > mboxes: > > description: > > List of phandle and mailbox channel specifiers. It should contain > > - exactly one or two mailboxes, one for transmitting messages("tx") > > - and another optional for receiving the notifications("rx") if supported. > > + exactly one, two or three mailboxes; the first one or two for transmitting > > + messages ("tx") and another optional ("rx") for receiving notifications > > + and delayed responses, if supported by the platform. > > + The number of mailboxes needed for transmitting messages depends on the > > + type of channels exposed by the specific underlying mailbox controller; > > + one single channel descriptor is enough if such channel is bidirectional, > > + while two channel descriptors are needed to represent the SCMI ("tx") > > + channel if the underlying mailbox channels are of unidirectional type. > > + The effective combination in numbers of mboxes and shmem descriptors let > > + the SCMI subsystem determine unambiguosly which type of SCMI channels are > > + made available by the underlying mailbox controller and how to use them. > > + 1 mbox / 1 shmem => SCMI TX over 1 mailbox bidirectional channel > > + 2 mbox / 2 shmem => SCMI TX and RX over 2 mailbox bidirectional channels > > + 2 mbox / 1 shmem => SCMI TX over 2 mailbox unidirectional channels > > + 3 mbox / 2 shmem => SCMI TX and RX over 3 mailbox unidirectional channels > > + Any other combination of mboxes and shmem is invalid. > > minItems: 1 > > - maxItems: 2 > > + maxItems: 3 > > Missing update to mbox-names. > Ah .. missed that since mbox-names is not marked as a required property in this binding. I'll add in V2. > > > > shmem: > > description: > > @@ -234,7 +248,7 @@ $defs: > > > > mboxes: > > minItems: 1 > > - maxItems: 2 > > + maxItems: 3 > > The same. How is it supposed to work? tx rx and that's it? > The logic is that SCMI transport can determine which type of channels (bidir vs unidir) you are using by looking at how many mboxes and how many shmem are defined as detailed in the description above. (not using mbox-names refs because was never marked as required so it would break backward compatibility starting to use that) I'll add a fix in V2 to fit mbox-names into this logic too. > > > > shmem: > > minItems: 1 > > @@ -393,6 +407,26 @@ examples: > > }; > > }; > > > > + - | > > + firmware { > > + scmi { > > + compatible = "arm,scmi"; > > + mboxes = <&mhu_U_tx 0 0>, <&mhu_U_rx 0 0>; > > + shmem = <&cpu_scp_lpri0>; > > + > > + #address-cells = <1>; > > + #size-cells = <0>; > > I don't think adding one more example with difference in only one piece > is needed here. > Mmm, I thought was sensible to add this example, given that a mailbox transport configuration for a mailbox exposing unidrectional channels is quite different from the usual bidirectional channel config already present in the pre-existent example. I'll add mbox-names into this example and see if I can change your mind...or I can then finally drop it. Thanks, Cristian
On 27/03/2023 17:27, Cristian Marussi wrote: >>> + - | >>> + firmware { >>> + scmi { >>> + compatible = "arm,scmi"; >>> + mboxes = <&mhu_U_tx 0 0>, <&mhu_U_rx 0 0>; >>> + shmem = <&cpu_scp_lpri0>; >>> + >>> + #address-cells = <1>; >>> + #size-cells = <0>; >> >> I don't think adding one more example with difference in only one piece >> is needed here. >> > > Mmm, I thought was sensible to add this example, given that a mailbox > transport configuration for a mailbox exposing unidrectional channels is > quite different from the usual bidirectional channel config already > present in the pre-existent example. > > I'll add mbox-names into this example and see if I can change your > mind...or I can then finally drop it. And what exactly this one more example changes? Does not validate different parts of the binding if only one property differs... Best regards, Krzysztof
On Tue, Mar 28, 2023 at 08:36:51AM +0200, Krzysztof Kozlowski wrote: > On 27/03/2023 17:27, Cristian Marussi wrote: > >>> + - | > >>> + firmware { > >>> + scmi { > >>> + compatible = "arm,scmi"; > >>> + mboxes = <&mhu_U_tx 0 0>, <&mhu_U_rx 0 0>; > >>> + shmem = <&cpu_scp_lpri0>; > >>> + > >>> + #address-cells = <1>; > >>> + #size-cells = <0>; > >> > >> I don't think adding one more example with difference in only one piece > >> is needed here. > >> > > > > Mmm, I thought was sensible to add this example, given that a mailbox > > transport configuration for a mailbox exposing unidrectional channels is > > quite different from the usual bidirectional channel config already > > present in the pre-existent example. > > > > I'll add mbox-names into this example and see if I can change your > > mind...or I can then finally drop it. > > And what exactly this one more example changes? Does not validate > different parts of the binding if only one property differs... Well it showcases how the extended new mboxes/shmem prop can be used in to support such unidirectional channels (which is pretty much different from the usual existing biridrectional synatx) ... anyway I never really thought as the examples in terms of validation really (and I am not saying that this is right eh) ... but more as an aid to help the unfortunate human being that has finally to write some DT based on this. Anyway since it does not seem appropriate, I'll just drop the whole example in V3, after waiting for some more (if any) feedback on the binding in general. Are the mbox-names fixes I added in V2 fine ? Thanks, Cristian
diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml index 2f7c51c75e85..9a7dc30e386f 100644 --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml @@ -63,10 +63,24 @@ properties: mboxes: description: List of phandle and mailbox channel specifiers. It should contain - exactly one or two mailboxes, one for transmitting messages("tx") - and another optional for receiving the notifications("rx") if supported. + exactly one, two or three mailboxes; the first one or two for transmitting + messages ("tx") and another optional ("rx") for receiving notifications + and delayed responses, if supported by the platform. + The number of mailboxes needed for transmitting messages depends on the + type of channels exposed by the specific underlying mailbox controller; + one single channel descriptor is enough if such channel is bidirectional, + while two channel descriptors are needed to represent the SCMI ("tx") + channel if the underlying mailbox channels are of unidirectional type. + The effective combination in numbers of mboxes and shmem descriptors let + the SCMI subsystem determine unambiguosly which type of SCMI channels are + made available by the underlying mailbox controller and how to use them. + 1 mbox / 1 shmem => SCMI TX over 1 mailbox bidirectional channel + 2 mbox / 2 shmem => SCMI TX and RX over 2 mailbox bidirectional channels + 2 mbox / 1 shmem => SCMI TX over 2 mailbox unidirectional channels + 3 mbox / 2 shmem => SCMI TX and RX over 3 mailbox unidirectional channels + Any other combination of mboxes and shmem is invalid. minItems: 1 - maxItems: 2 + maxItems: 3 shmem: description: @@ -234,7 +248,7 @@ $defs: mboxes: minItems: 1 - maxItems: 2 + maxItems: 3 shmem: minItems: 1 @@ -393,6 +407,26 @@ examples: }; }; + - | + firmware { + scmi { + compatible = "arm,scmi"; + mboxes = <&mhu_U_tx 0 0>, <&mhu_U_rx 0 0>; + shmem = <&cpu_scp_lpri0>; + + #address-cells = <1>; + #size-cells = <0>; + + scmi_dvfs_2: protocol@13 { + reg = <0x13>; + #clock-cells = <1>; + + mboxes = <&mhu_U_tx 1 0>, <&mhu_U_rx 1 0>, <&mhu_U_rx 1 1>; + shmem = <&cpu_scp_hpri0>, <&cpu_scp_hpri1>; + }; + }; + }; + - | firmware { scmi {