| Message ID | 20240912170025.455167-3-valentina.fernandezalanis@microchip.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add Microchip IPC mailbox and remoteproc support | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On Thu, Sep 12, 2024 at 06:00:22PM +0100, Valentina Fernandez wrote: > Add a dt-binding for the Microchip Inter-Processor Communication (IPC) > mailbox controller. Before anyone else gets here, there's an erroneous "driver" in $subject :) > Signed-off-by: Valentina Fernandez <valentina.fernandezalanis@microchip.com> > --- > .../bindings/mailbox/microchip,sbi-ipc.yaml | 115 ++++++++++++++++++ > 1 file changed, 115 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mailbox/microchip,sbi-ipc.yaml > > diff --git a/Documentation/devicetree/bindings/mailbox/microchip,sbi-ipc.yaml b/Documentation/devicetree/bindings/mailbox/microchip,sbi-ipc.yaml > new file mode 100644 > index 000000000000..dc2cbd5eb28f > --- /dev/null > +++ b/Documentation/devicetree/bindings/mailbox/microchip,sbi-ipc.yaml > @@ -0,0 +1,115 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mailbox/microchip,sbi-ipc.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Microchip Inter-processor communication (IPC) mailbox controller > + > +maintainers: > + - Valentina Fernandez <valentina.fernandezalanis@microchip.com> > + > +description: > + The Microchip Inter-processor Communication (IPC) facilitates > + message passing between processors using an interrupt signaling > + mechanism. > + This SBI interface is compatible with the Mi-V Inter-hart > + Communication (IHC) IP. > + The microchip,sbi-ipc compatible string is inteded for use by software > + running in supervisor privileged mode (s-mode). The SoC-specific > + compatibles are inteded for use by the SBI implementation in machine > + mode (m-mode). And it might be worth me pointing out to the !riscv folks that "SBI implementation in machine mode" means that the "microchip,miv-ihc-rtl-v2" compatible is intended for use by firmware only. Cheers, Conor. > + > +properties: > + compatible: > + enum: > + - microchip,sbi-ipc > + - microchip,miv-ihc-rtl-v2 > + > + reg: > + maxItems: 1 > + > + interrupts: > + minItems: 1 > + maxItems: 5 > + > + interrupt-names: > + minItems: 1 > + maxItems: 5 > + > + "#mbox-cells": > + description: > + For the SBI "device", the cell represents the global "logical" channel IDs. > + The meaning of channel IDs are platform firmware dependent. The > + SoC-specific compatibles are intended for use by the SBI implementation, > + rather than s-mode software. There the cell would represent the physical > + channel and do not vary depending on platform firmware. > + const: 1 > + > + microchip,ihc-chan-disabled-mask: > + description: > + Represents the enable/disable state of the bi-directional IHC channels > + within the MIV-IHC IP configuration. The mask is a 16-bit value, but only > + the first 15 bits are utilized.Each of the bits corresponds to > + one of the 15 IHC channels. > + A bit set to '1' indicates that the corresponding channel is disabled, > + and any read or write operations to that channel will return zero. > + A bit set to '0' indicates that the corresponding channel is enabled > + and will be accessible through its dedicated address range registers. > + The remaining bit of the 16-bit mask is reserved and should be ignored. > + The actual enable/disable state of each channel is determined by the > + IP block’s configuration. > + $ref: /schemas/types.yaml#/definitions/uint16 > + default: 0 > + > +required: > + - compatible > + - interrupts > + - interrupt-names > + - "#mbox-cells" > + > +additionalProperties: false > + > +allOf: > + - if: > + properties: > + compatible: > + contains: > + const: microchip,sbi-ipc > + then: > + properties: > + reg: false > + else: > + required: > + - reg > + > + - if: > + properties: > + compatible: > + contains: > + const: microchip,miv-ihc-rtl-v2 > + then: > + properties: > + interrupt-names: > + items: > + pattern: "^hart-[0-5]+$" > + > +examples: > + - | > + mailbox { > + compatible = "microchip,sbi-ipc"; > + interrupt-parent = <&plic>; > + interrupts = <180>, <179>, <178>; > + interrupt-names = "hart-1", "hart-2", "hart-3"; > + #mbox-cells = <1>; > + }; > + - | > + mailbox@50000000 { > + compatible = "microchip,miv-ihc-rtl-v2"; > + microchip,ihc-chan-disabled-mask= /bits/ 16 <0>; > + reg = <0x50000000 0x1C000>; > + interrupt-parent = <&plic>; > + interrupts = <180>, <179>, <178>; > + interrupt-names = "hart-1", "hart-2", "hart-3"; > + #mbox-cells = <1>; > + }; > -- > 2.34.1 >
Hi Valentina, On 2024-09-12 12:00 PM, Valentina Fernandez wrote: > Add a dt-binding for the Microchip Inter-Processor Communication (IPC) > mailbox controller. > > Signed-off-by: Valentina Fernandez <valentina.fernandezalanis@microchip.com> > --- > .../bindings/mailbox/microchip,sbi-ipc.yaml | 115 ++++++++++++++++++ > 1 file changed, 115 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mailbox/microchip,sbi-ipc.yaml > > diff --git a/Documentation/devicetree/bindings/mailbox/microchip,sbi-ipc.yaml b/Documentation/devicetree/bindings/mailbox/microchip,sbi-ipc.yaml > new file mode 100644 > index 000000000000..dc2cbd5eb28f > --- /dev/null > +++ b/Documentation/devicetree/bindings/mailbox/microchip,sbi-ipc.yaml > @@ -0,0 +1,115 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mailbox/microchip,sbi-ipc.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Microchip Inter-processor communication (IPC) mailbox controller > + > +maintainers: > + - Valentina Fernandez <valentina.fernandezalanis@microchip.com> > + > +description: > + The Microchip Inter-processor Communication (IPC) facilitates > + message passing between processors using an interrupt signaling > + mechanism. > + This SBI interface is compatible with the Mi-V Inter-hart > + Communication (IHC) IP. > + The microchip,sbi-ipc compatible string is inteded for use by software > + running in supervisor privileged mode (s-mode). The SoC-specific > + compatibles are inteded for use by the SBI implementation in machine > + mode (m-mode). There is a lot of conditional logic in this binding for how small it is. Would it make sense to split this into two separate bindings? For example, with the current binding microchip,ihc-chan-disabled-mask is allowed for the SBI interface, but doesn't look like it belongs there. > + > +properties: > + compatible: > + enum: > + - microchip,sbi-ipc > + - microchip,miv-ihc-rtl-v2 > + > + reg: > + maxItems: 1 > + > + interrupts: > + minItems: 1 > + maxItems: 5 > + > + interrupt-names: > + minItems: 1 > + maxItems: 5 > + > + "#mbox-cells": > + description: > + For the SBI "device", the cell represents the global "logical" channel IDs. > + The meaning of channel IDs are platform firmware dependent. The > + SoC-specific compatibles are intended for use by the SBI implementation, > + rather than s-mode software. There the cell would represent the physical > + channel and do not vary depending on platform firmware. > + const: 1 > + > + microchip,ihc-chan-disabled-mask: > + description: > + Represents the enable/disable state of the bi-directional IHC channels > + within the MIV-IHC IP configuration. The mask is a 16-bit value, but only > + the first 15 bits are utilized.Each of the bits corresponds to > + one of the 15 IHC channels. > + A bit set to '1' indicates that the corresponding channel is disabled, > + and any read or write operations to that channel will return zero. > + A bit set to '0' indicates that the corresponding channel is enabled > + and will be accessible through its dedicated address range registers. > + The remaining bit of the 16-bit mask is reserved and should be ignored. > + The actual enable/disable state of each channel is determined by the > + IP block’s configuration. > + $ref: /schemas/types.yaml#/definitions/uint16 > + default: 0 > + > +required: > + - compatible > + - interrupts > + - interrupt-names > + - "#mbox-cells" > + > +additionalProperties: false > + > +allOf: > + - if: > + properties: > + compatible: > + contains: > + const: microchip,sbi-ipc > + then: > + properties: > + reg: false > + else: > + required: > + - reg > + > + - if: > + properties: > + compatible: > + contains: > + const: microchip,miv-ihc-rtl-v2 > + then: > + properties: > + interrupt-names: > + items: > + pattern: "^hart-[0-5]+$" The driver in patch 3 uses this pattern for the SBI interface, so should it apply there as well? Regards, Samuel > + > +examples: > + - | > + mailbox { > + compatible = "microchip,sbi-ipc"; > + interrupt-parent = <&plic>; > + interrupts = <180>, <179>, <178>; > + interrupt-names = "hart-1", "hart-2", "hart-3"; > + #mbox-cells = <1>; > + }; > + - | > + mailbox@50000000 { > + compatible = "microchip,miv-ihc-rtl-v2"; > + microchip,ihc-chan-disabled-mask= /bits/ 16 <0>; > + reg = <0x50000000 0x1C000>; > + interrupt-parent = <&plic>; > + interrupts = <180>, <179>, <178>; > + interrupt-names = "hart-1", "hart-2", "hart-3"; > + #mbox-cells = <1>; > + };
On Thu, Sep 12, 2024 at 04:23:44PM -0500, Samuel Holland wrote: > Hi Valentina, > > On 2024-09-12 12:00 PM, Valentina Fernandez wrote: > > Add a dt-binding for the Microchip Inter-Processor Communication (IPC) > > mailbox controller. > > > > Signed-off-by: Valentina Fernandez <valentina.fernandezalanis@microchip.com> > > --- > > .../bindings/mailbox/microchip,sbi-ipc.yaml | 115 ++++++++++++++++++ > > 1 file changed, 115 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/mailbox/microchip,sbi-ipc.yaml > > > > diff --git a/Documentation/devicetree/bindings/mailbox/microchip,sbi-ipc.yaml b/Documentation/devicetree/bindings/mailbox/microchip,sbi-ipc.yaml > > new file mode 100644 > > index 000000000000..dc2cbd5eb28f > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mailbox/microchip,sbi-ipc.yaml > > @@ -0,0 +1,115 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/mailbox/microchip,sbi-ipc.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Microchip Inter-processor communication (IPC) mailbox controller > > + > > +maintainers: > > + - Valentina Fernandez <valentina.fernandezalanis@microchip.com> > > + > > +description: > > + The Microchip Inter-processor Communication (IPC) facilitates > > + message passing between processors using an interrupt signaling > > + mechanism. > > + This SBI interface is compatible with the Mi-V Inter-hart > > + Communication (IHC) IP. > > + The microchip,sbi-ipc compatible string is inteded for use by software > > + running in supervisor privileged mode (s-mode). The SoC-specific > > + compatibles are inteded for use by the SBI implementation in machine > > + mode (m-mode). > > There is a lot of conditional logic in this binding for how small it is. Would > it make sense to split this into two separate bindings? For example, with the > current binding microchip,ihc-chan-disabled-mask is allowed for the SBI > interface, but doesn't look like it belongs there. I dunno. Part of me says that because this is two compatibles for the same piece of hardware (the choice depending on which programming model you use) they should be documented together. The other part of me is of the opinion that they effectively describe different things, given one describes the hardware and the other describes a firmware interface that may have any sort of hardware backing it. I suppose it's more of a problem for "us" (that being me/Rob/Krzysztof) than for Valentina, and how to handle firmware interfaces to hardware like this is one of the topics that's planned for Krzysztof's devicetree BoF session at LPC. Cheers, Conor.
On Mon, Sep 16, 2024 at 05:31:36PM +0100, Conor Dooley wrote: > On Thu, Sep 12, 2024 at 04:23:44PM -0500, Samuel Holland wrote: > > Hi Valentina, > > > > On 2024-09-12 12:00 PM, Valentina Fernandez wrote: > > > Add a dt-binding for the Microchip Inter-Processor Communication (IPC) > > > mailbox controller. > > > > > > Signed-off-by: Valentina Fernandez <valentina.fernandezalanis@microchip.com> > > > --- > > > .../bindings/mailbox/microchip,sbi-ipc.yaml | 115 ++++++++++++++++++ > > > 1 file changed, 115 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/mailbox/microchip,sbi-ipc.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/mailbox/microchip,sbi-ipc.yaml b/Documentation/devicetree/bindings/mailbox/microchip,sbi-ipc.yaml > > > new file mode 100644 > > > index 000000000000..dc2cbd5eb28f > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/mailbox/microchip,sbi-ipc.yaml > > > @@ -0,0 +1,115 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/mailbox/microchip,sbi-ipc.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Microchip Inter-processor communication (IPC) mailbox controller > > > + > > > +maintainers: > > > + - Valentina Fernandez <valentina.fernandezalanis@microchip.com> > > > + > > > +description: > > > + The Microchip Inter-processor Communication (IPC) facilitates > > > + message passing between processors using an interrupt signaling > > > + mechanism. > > > + This SBI interface is compatible with the Mi-V Inter-hart > > > + Communication (IHC) IP. > > > + The microchip,sbi-ipc compatible string is inteded for use by software > > > + running in supervisor privileged mode (s-mode). The SoC-specific > > > + compatibles are inteded for use by the SBI implementation in machine > > > + mode (m-mode). > > > > There is a lot of conditional logic in this binding for how small it is. Would > > it make sense to split this into two separate bindings? For example, with the > > current binding microchip,ihc-chan-disabled-mask is allowed for the SBI > > interface, but doesn't look like it belongs there. > > I dunno. Part of me says that because this is two compatibles for the > same piece of hardware (the choice depending on which programming model > you use) they should be documented together. The other part of me is of > the opinion that they effectively describe different things, given one > describes the hardware and the other describes a firmware interface that > may have any sort of hardware backing it. > > I suppose it's more of a problem for "us" (that being me/Rob/Krzysztof) > than for Valentina, and how to handle firmware interfaces to hardware > like this is one of the topics that's planned for Krzysztof's devicetree > BoF session at LPC. If how the client interacts with the device is fundamentally different, then I think different compatibles is fine. Rob
On Wed, Sep 18, 2024 at 10:35:58AM -0500, Rob Herring wrote: > On Mon, Sep 16, 2024 at 05:31:36PM +0100, Conor Dooley wrote: > > On Thu, Sep 12, 2024 at 04:23:44PM -0500, Samuel Holland wrote: > > > Hi Valentina, > > > > > > On 2024-09-12 12:00 PM, Valentina Fernandez wrote: > > > > Add a dt-binding for the Microchip Inter-Processor Communication (IPC) > > > > mailbox controller. > > > > > > > > Signed-off-by: Valentina Fernandez <valentina.fernandezalanis@microchip.com> > > > > --- > > > > .../bindings/mailbox/microchip,sbi-ipc.yaml | 115 ++++++++++++++++++ > > > > 1 file changed, 115 insertions(+) > > > > create mode 100644 Documentation/devicetree/bindings/mailbox/microchip,sbi-ipc.yaml > > > > > > > > diff --git a/Documentation/devicetree/bindings/mailbox/microchip,sbi-ipc.yaml b/Documentation/devicetree/bindings/mailbox/microchip,sbi-ipc.yaml > > > > new file mode 100644 > > > > index 000000000000..dc2cbd5eb28f > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/mailbox/microchip,sbi-ipc.yaml > > > > @@ -0,0 +1,115 @@ > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > +%YAML 1.2 > > > > +--- > > > > +$id: http://devicetree.org/schemas/mailbox/microchip,sbi-ipc.yaml# > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > + > > > > +title: Microchip Inter-processor communication (IPC) mailbox controller > > > > + > > > > +maintainers: > > > > + - Valentina Fernandez <valentina.fernandezalanis@microchip.com> > > > > + > > > > +description: > > > > + The Microchip Inter-processor Communication (IPC) facilitates > > > > + message passing between processors using an interrupt signaling > > > > + mechanism. > > > > + This SBI interface is compatible with the Mi-V Inter-hart > > > > + Communication (IHC) IP. > > > > + The microchip,sbi-ipc compatible string is inteded for use by software > > > > + running in supervisor privileged mode (s-mode). The SoC-specific > > > > + compatibles are inteded for use by the SBI implementation in machine > > > > + mode (m-mode). > > > > > > There is a lot of conditional logic in this binding for how small it is. Would > > > it make sense to split this into two separate bindings? For example, with the > > > current binding microchip,ihc-chan-disabled-mask is allowed for the SBI > > > interface, but doesn't look like it belongs there. > > > > I dunno. Part of me says that because this is two compatibles for the > > same piece of hardware (the choice depending on which programming model > > you use) they should be documented together. The other part of me is of > > the opinion that they effectively describe different things, given one > > describes the hardware and the other describes a firmware interface that > > may have any sort of hardware backing it. > > > > I suppose it's more of a problem for "us" (that being me/Rob/Krzysztof) > > than for Valentina, and how to handle firmware interfaces to hardware > > like this is one of the topics that's planned for Krzysztof's devicetree > > BoF session at LPC. > > If how the client interacts with the device is fundamentally different, > then I think different compatibles is fine. It wasn't about different compatibles (which I think are non-debatable here) it's whether or not the different compatibles should be in their own binding files.
diff --git a/Documentation/devicetree/bindings/mailbox/microchip,sbi-ipc.yaml b/Documentation/devicetree/bindings/mailbox/microchip,sbi-ipc.yaml new file mode 100644 index 000000000000..dc2cbd5eb28f --- /dev/null +++ b/Documentation/devicetree/bindings/mailbox/microchip,sbi-ipc.yaml @@ -0,0 +1,115 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mailbox/microchip,sbi-ipc.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Microchip Inter-processor communication (IPC) mailbox controller + +maintainers: + - Valentina Fernandez <valentina.fernandezalanis@microchip.com> + +description: + The Microchip Inter-processor Communication (IPC) facilitates + message passing between processors using an interrupt signaling + mechanism. + This SBI interface is compatible with the Mi-V Inter-hart + Communication (IHC) IP. + The microchip,sbi-ipc compatible string is inteded for use by software + running in supervisor privileged mode (s-mode). The SoC-specific + compatibles are inteded for use by the SBI implementation in machine + mode (m-mode). + +properties: + compatible: + enum: + - microchip,sbi-ipc + - microchip,miv-ihc-rtl-v2 + + reg: + maxItems: 1 + + interrupts: + minItems: 1 + maxItems: 5 + + interrupt-names: + minItems: 1 + maxItems: 5 + + "#mbox-cells": + description: + For the SBI "device", the cell represents the global "logical" channel IDs. + The meaning of channel IDs are platform firmware dependent. The + SoC-specific compatibles are intended for use by the SBI implementation, + rather than s-mode software. There the cell would represent the physical + channel and do not vary depending on platform firmware. + const: 1 + + microchip,ihc-chan-disabled-mask: + description: + Represents the enable/disable state of the bi-directional IHC channels + within the MIV-IHC IP configuration. The mask is a 16-bit value, but only + the first 15 bits are utilized.Each of the bits corresponds to + one of the 15 IHC channels. + A bit set to '1' indicates that the corresponding channel is disabled, + and any read or write operations to that channel will return zero. + A bit set to '0' indicates that the corresponding channel is enabled + and will be accessible through its dedicated address range registers. + The remaining bit of the 16-bit mask is reserved and should be ignored. + The actual enable/disable state of each channel is determined by the + IP block’s configuration. + $ref: /schemas/types.yaml#/definitions/uint16 + default: 0 + +required: + - compatible + - interrupts + - interrupt-names + - "#mbox-cells" + +additionalProperties: false + +allOf: + - if: + properties: + compatible: + contains: + const: microchip,sbi-ipc + then: + properties: + reg: false + else: + required: + - reg + + - if: + properties: + compatible: + contains: + const: microchip,miv-ihc-rtl-v2 + then: + properties: + interrupt-names: + items: + pattern: "^hart-[0-5]+$" + +examples: + - | + mailbox { + compatible = "microchip,sbi-ipc"; + interrupt-parent = <&plic>; + interrupts = <180>, <179>, <178>; + interrupt-names = "hart-1", "hart-2", "hart-3"; + #mbox-cells = <1>; + }; + - | + mailbox@50000000 { + compatible = "microchip,miv-ihc-rtl-v2"; + microchip,ihc-chan-disabled-mask= /bits/ 16 <0>; + reg = <0x50000000 0x1C000>; + interrupt-parent = <&plic>; + interrupts = <180>, <179>, <178>; + interrupt-names = "hart-1", "hart-2", "hart-3"; + #mbox-cells = <1>; + };
Add a dt-binding for the Microchip Inter-Processor Communication (IPC) mailbox controller. Signed-off-by: Valentina Fernandez <valentina.fernandezalanis@microchip.com> --- .../bindings/mailbox/microchip,sbi-ipc.yaml | 115 ++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 Documentation/devicetree/bindings/mailbox/microchip,sbi-ipc.yaml