| Message ID | 1506604306-20739-4-git-send-email-sudeep.holla@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,01/22] dt-bindings: mailbox: add support for mailbox client shared memory | expand |
On 06/10/17 00:20, Rob Herring wrote: > On Thu, Sep 28, 2017 at 02:11:27PM +0100, Sudeep Holla wrote: >> This patch adds ARM MHU specific mailbox client bindings to support >> SCMI. Since SCMI specification just requires doorbell mechanism from >> mailbox controllers, we add mailbox data to specify the doorbell bit(s). >> >> Cc: Rob Herring <robh+dt@kernel.org> >> Cc: Mark Rutland <mark.rutland@arm.com> >> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> >> --- >> .../devicetree/bindings/arm/arm,mhu-scmi.txt | 19 +++++++++++++++++++ >> 1 file changed, 19 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/arm/arm,mhu-scmi.txt >> >> diff --git a/Documentation/devicetree/bindings/arm/arm,mhu-scmi.txt b/Documentation/devicetree/bindings/arm/arm,mhu-scmi.txt >> new file mode 100644 >> index 000000000000..8c106f1cdeb8 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/arm/arm,mhu-scmi.txt >> @@ -0,0 +1,19 @@ >> +ARM MHU mailbox client bindings for SCMI Message Protocol >> +---------------------------------------------------------- >> + >> +This binding is intended to define the ARM MHU specific extensions to >> +the generic SCMI bindings[2]. >> + >> +Required properties: >> + >> +The scmi node with the following properties shall be under the /firmware/ node. >> + >> +- compatible : shall be "arm,scmi" and "arm,mhu-scmi" > > Most specific first. > Ah right, sorry for missing that. >> +- mbox-data : For each phandle listed in mboxes property, an unsigned 32-bit >> + data as expected by the mailbox controller > > Shouldn't that be cells as part of mboxes property? > Yes, that's what I proposed with my ARM MHU doorbell bindings[1]. Since Jassi rejected that and asked to make it part client. But I agree with you comment. Even Arnd had similar opinion. -- Regards, Sudeep [1] https://patchwork.kernel.org/patch/9745683/
On Fri, Oct 6, 2017 at 6:01 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote: > On Fri, Oct 6, 2017 at 4:50 AM, Rob Herring <robh@kernel.org> wrote: >> On Thu, Sep 28, 2017 at 02:11:27PM +0100, Sudeep Holla wrote: >>> This patch adds ARM MHU specific mailbox client bindings to support >>> SCMI. Since SCMI specification just requires doorbell mechanism from >>> mailbox controllers, we add mailbox data to specify the doorbell bit(s). >>> >>> Cc: Rob Herring <robh+dt@kernel.org> >>> Cc: Mark Rutland <mark.rutland@arm.com> >>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> >>> --- >>> .../devicetree/bindings/arm/arm,mhu-scmi.txt | 19 +++++++++++++++++++ >>> 1 file changed, 19 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/arm/arm,mhu-scmi.txt >>> >>> diff --git a/Documentation/devicetree/bindings/arm/arm,mhu-scmi.txt b/Documentation/devicetree/bindings/arm/arm,mhu-scmi.txt >>> new file mode 100644 >>> index 000000000000..8c106f1cdeb8 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/arm/arm,mhu-scmi.txt >>> @@ -0,0 +1,19 @@ >>> +ARM MHU mailbox client bindings for SCMI Message Protocol >>> +---------------------------------------------------------- >>> + >>> +This binding is intended to define the ARM MHU specific extensions to >>> +the generic SCMI bindings[2]. >>> + >>> +Required properties: >>> + >>> +The scmi node with the following properties shall be under the /firmware/ node. >>> + >>> +- compatible : shall be "arm,scmi" and "arm,mhu-scmi" >> >> Most specific first. >> >>> +- mbox-data : For each phandle listed in mboxes property, an unsigned 32-bit >>> + data as expected by the mailbox controller >> >> Shouldn't that be cells as part of mboxes property? >> > A MHU client can send any number of commands (such u32 values) over a channel. > This client (SCMI) sends just one command over a channel, but other > clients may/do send two or more. Okay, then I guess I don't understand why this is in DT. Rob
On Fri, Oct 6, 2017 at 9:26 PM, Jassi Brar <jassisinghbrar@gmail.com> wrote: > On Fri, Oct 6, 2017 at 9:24 PM, Rob Herring <robh@kernel.org> wrote: >> On Fri, Oct 6, 2017 at 6:01 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote: >>> On Fri, Oct 6, 2017 at 4:50 AM, Rob Herring <robh@kernel.org> wrote: >>>> On Thu, Sep 28, 2017 at 02:11:27PM +0100, Sudeep Holla wrote: > >>>> >>>>> +- mbox-data : For each phandle listed in mboxes property, an unsigned 32-bit >>>>> + data as expected by the mailbox controller >>>> >>>> Shouldn't that be cells as part of mboxes property? >>>> >>> A MHU client can send any number of commands (such u32 values) over a channel. >>> This client (SCMI) sends just one command over a channel, but other >>> clients may/do send two or more. The above definition doesn't support 2 or more as it is 1-1 with channels. >> Okay, then I guess I don't understand why this is in DT. >> > Yeah the client has to provide code (u32 value) for the commands it > sends, and that value is going to be platform specific. For example, > on Juno the ITS_AN_SCMI_COMMAND may be defined as BIT(7) while on my > platform it may be 0x4567 > > For MHU based platforms, it becomes easy if the u32 is passed from DT. > And that should be ok since that is like a h/w parameter - a value > chosen/expected by the remote firmware. Could it ever be more than 1 cell? I guess being in DT is fine, but I'm still not sure about the naming. The current name suggests it is part of the mbox binding. Do we want that or should it be SCMI specific? Then "data" is vague. Perhaps "scmi-commands"? Rob
On 09/10/17 23:57, Rob Herring wrote: > On Mon, Oct 9, 2017 at 9:46 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote: >> On Mon, Oct 9, 2017 at 7:22 PM, Rob Herring <robh@kernel.org> wrote: >>> On Fri, Oct 6, 2017 at 9:26 PM, Jassi Brar <jassisinghbrar@gmail.com> wrote: >>>> On Fri, Oct 6, 2017 at 9:24 PM, Rob Herring <robh@kernel.org> wrote: >>>>> On Fri, Oct 6, 2017 at 6:01 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote: >>>>>> On Fri, Oct 6, 2017 at 4:50 AM, Rob Herring <robh@kernel.org> wrote: >>>>>>> On Thu, Sep 28, 2017 at 02:11:27PM +0100, Sudeep Holla wrote: >>>> >>>>>>> >>>>>>>> +- mbox-data : For each phandle listed in mboxes property, an unsigned 32-bit >>>>>>>> + data as expected by the mailbox controller >>>>>>> >>>>>>> Shouldn't that be cells as part of mboxes property? >>>>>>> >>>>>> A MHU client can send any number of commands (such u32 values) over a channel. >>>>>> This client (SCMI) sends just one command over a channel, but other >>>>>> clients may/do send two or more. >>> >>> The above definition doesn't support 2 or more as it is 1-1 with channels. >>> >> I thought you suggested to make controller driver accept the command >> as another cell in client's mboxes property. >> Which we can't do. > > Yes, agreed. But I'm wondering since a client may need more than one, > how would that be expressed? > >>>>> Okay, then I guess I don't understand why this is in DT. >>>>> >>>> Yeah the client has to provide code (u32 value) for the commands it >>>> sends, and that value is going to be platform specific. For example, >>>> on Juno the ITS_AN_SCMI_COMMAND may be defined as BIT(7) while on my >>>> platform it may be 0x4567 >>>> >>>> For MHU based platforms, it becomes easy if the u32 is passed from DT. >>>> And that should be ok since that is like a h/w parameter - a value >>>> chosen/expected by the remote firmware. >>> >>> Could it ever be more than 1 cell? >>> >> SCMI sends sub-commands via SHMEM, so it is always going to be 1cell for _scmi_. >> However many firmwares are unlikely to use just one command over a >> channel - say, the protocol is trivial or the linux and remote have no >> SHMEM. > > I'd hope the normal case is not enumerating commands and sub-commands > in DT and this is special case of a "generic" protocol with platform > specific aspects. It could be solved with a specific compatible for > each platform/implementation. We'll probably regret not doing that, > but I'm going to pretend that this time SoC vendors won't mess it up. > Just to align on the same page, I would like to define the terms used above in context of SCMI: 1. commands : this is platform specific "command" to trigger/signal the firmware that SCMI packet is sent. This is always platform or controller specific and is not part of the specification. It's just like doorbell but since MHU can send 32-bit data, we need a way to specify the exact bit as 32-bit data with just one bit set. 2. sub-commands : It is SCMI packet that includes the actual command defined in the SCMI specification. There are sent in SHMEM as Jassi mentioned above. -- Regards, Sudeep
diff --git a/Documentation/devicetree/bindings/arm/arm,mhu-scmi.txt b/Documentation/devicetree/bindings/arm/arm,mhu-scmi.txt new file mode 100644 index 000000000000..8c106f1cdeb8 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/arm,mhu-scmi.txt @@ -0,0 +1,19 @@ +ARM MHU mailbox client bindings for SCMI Message Protocol +---------------------------------------------------------- + +This binding is intended to define the ARM MHU specific extensions to +the generic SCMI bindings[2]. + +Required properties: + +The scmi node with the following properties shall be under the /firmware/ node. + +- compatible : shall be "arm,scmi" and "arm,mhu-scmi" +- mbox-data : For each phandle listed in mboxes property, an unsigned 32-bit + data as expected by the mailbox controller + +See [1] for details on all other required/optional properties of the generic +mailbox controller and [2] for generic SCMI bindings. + +[1] Documentation/devicetree/bindings/mailbox/mailbox.txt +[2] Documentation/devicetree/bindings/arm/arm,scmi.txt
This patch adds ARM MHU specific mailbox client bindings to support SCMI. Since SCMI specification just requires doorbell mechanism from mailbox controllers, we add mailbox data to specify the doorbell bit(s). Cc: Rob Herring <robh+dt@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- .../devicetree/bindings/arm/arm,mhu-scmi.txt | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/arm,mhu-scmi.txt -- 2.7.4