From: Hari Nagalla <[email protected]>
K3 AM64x SoC has a Cortex M4F subsystem in the MCU voltage domain.
The remote processor's life cycle management and IPC mechanisms are
similar across the R5F and M4F cores from remote processor driver
point of view. However, there are subtle differences in image loading
and starting the M4F subsystems.
The YAML binding document provides the various node properties to be
configured by the consumers of the M4F subsystem.
Signed-off-by: Hari Nagalla <[email protected]>
[Martyn Welch: Amended as per review comments and to pass DT tests]
Signed-off-by: Martyn Welch <[email protected]>
---
Changes since v1:
- Spelling corrections
- Corrected to pass DT checks
Changes since v2:
- Missed spelling correction to commit message
Note: The only review comment that I don't see directly addressed is the
lack of description of `ti,sci`, `ti,sci-dev-id` and
`ti,sci-proc-ids`. A reference has been added to
`/schemas/arm/keystone/ti,k3-sci-common.yaml#` where they are
described. I believe this is the correct approach, please advise if
that is not the case.
.../bindings/remoteproc/ti,k3-m4f-rproc.yaml | 158 ++++++++++++++++++
1 file changed, 158 insertions(+)
create mode 100644 Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml
diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml
new file mode 100644
index 000000000000..1b38df0be2e6
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml
@@ -0,0 +1,158 @@
+# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/remoteproc/ti,k3-m4f-rproc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI K3 M4F processor subsystems
+
+maintainers:
+ - Hari Nagalla <[email protected]>
+
+description: |
+ Some K3 family SoCs have Arm Cortex M4F cores. AM64x is a SoC in K3
+ family with a M4F core. Typically safety oriented applications may use
+ the M4F core in isolation without an IPC. Where as some industrial and
+ home automation applications, may use the M4F core as a remote processor
+ with IPC communications.
+
+$ref: /schemas/arm/keystone/ti,k3-sci-common.yaml#
+
+properties:
+ $nodename:
+ pattern: "^m4fss(@.*)?"
+
+ compatible:
+ enum:
+ - ti,am64-m4fss
+
+ power-domains:
+ description: |
+ Should contain a phandle to a PM domain provider node and an args
+ specifier containing the M4FSS device id value.
+ maxItems: 1
+
+ "#address-cells":
+ const: 2
+
+ "#size-cells":
+ const: 2
+
+ reg:
+ items:
+ - description: Address and Size of the IRAM internal memory region
+ - description: Address and Size of the DRAM internal memory region
+
+ reg-names:
+ items:
+ - const: iram
+ - const: dram
+
+ resets:
+ description: |
+ Should contain the phandle to the reset controller node managing the
+ local resets for this device, and a reset specifier.
+ maxItems: 1
+
+ firmware-name:
+ description: |
+ Should contain the name of the default firmware image
+ file located on the firmware search path
+
+ mboxes:
+ description: |
+ OMAP Mailbox specifier denoting the sub-mailbox, to be used for
+ communication with the remote processor. This property should match
+ with the sub-mailbox node used in the firmware image.
+ maxItems: 1
+
+ memory-region:
+ description: |
+ phandle to the reserved memory nodes to be associated with the
+ remoteproc device. There should be at least two reserved memory nodes
+ defined. The reserved memory nodes should be carveout nodes, and
+ should be defined with a "no-map" property as per the bindings in
+ Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
+ minItems: 2
+ maxItems: 8
+ items:
+ - description: region used for dynamic DMA allocations like vrings and
+ vring buffers
+ - description: region reserved for firmware image sections
+ additionalItems: true
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - ti,sci
+ - ti,sci-dev-id
+ - ti,sci-proc-ids
+ - resets
+ - firmware-name
+ - mboxes
+ - memory-region
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ reserved-memory {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ mcu_m4fss_dma_memory_region: m4f-dma-memory@9cb00000 {
+ compatible = "shared-dma-pool";
+ reg = <0x00 0x9cb00000 0x00 0x100000>;
+ no-map;
+ };
+
+ mcu_m4fss_memory_region: m4f-memory@9cc00000 {
+ compatible = "shared-dma-pool";
+ reg = <0x00 0x9cc00000 0x00 0xe00000>;
+ no-map;
+ };
+ };
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ mailbox0_cluster0: mailbox-0 {
+ #mbox-cells = <1>;
+
+ mbox_m4_0: mbox-m4-0 {
+ ti,mbox-rx = <0 0 0>;
+ ti,mbox-tx = <1 0 0>;
+ };
+ };
+
+ bus@f0000 {
+ compatible = "simple-bus";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges = <0x00 0x04000000 0x00 0x04000000 0x00 0x01ff1400>;
+
+ bus@4000000 {
+ compatible = "simple-bus";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges = <0x00 0x04000000 0x00 0x04000000 0x00 0x01ff1400>;
+
+ mcu_m4fss: m4fss@5000000 {
+ compatible = "ti,am64-m4fss";
+ reg = <0x00 0x5000000 0x00 0x30000>,
+ <0x00 0x5040000 0x00 0x10000>;
+ reg-names = "iram", "dram";
+ ti,sci = <&dmsc>;
+ ti,sci-dev-id = <9>;
+ ti,sci-proc-ids = <0x18 0xff>;
+ resets = <&k3_reset 9 1>;
+ firmware-name = "am62-mcu-m4f0_0-fw";
+ mboxes = <&mailbox0_cluster0 &mbox_m4_0>;
+ memory-region = <&mcu_m4fss_dma_memory_region>,
+ <&mcu_m4fss_memory_region>;
+ };
+ };
+ };
+ };
--
2.39.1
On 02/03/2023 18:14, Martyn Welch wrote:
> From: Hari Nagalla <[email protected]>
Subject: drop second/last, redundant "bindings for". The "dt-bindings"
prefix is already stating that these are bindings.
>
> K3 AM64x SoC has a Cortex M4F subsystem in the MCU voltage domain.
> The remote processor's life cycle management and IPC mechanisms are
> similar across the R5F and M4F cores from remote processor driver
> point of view. However, there are subtle differences in image loading
> and starting the M4F subsystems.
>
> The YAML binding document provides the various node properties to be
> configured by the consumers of the M4F subsystem.
>
> Signed-off-by: Hari Nagalla <[email protected]>
> [Martyn Welch: Amended as per review comments and to pass DT tests]
> Signed-off-by: Martyn Welch <[email protected]>
> ---
>
> Changes since v1:
> - Spelling corrections
> - Corrected to pass DT checks
>
> Changes since v2:
> - Missed spelling correction to commit message
>
> Note: The only review comment that I don't see directly addressed is the
> lack of description of `ti,sci`, `ti,sci-dev-id` and
> `ti,sci-proc-ids`. A reference has been added to
> `/schemas/arm/keystone/ti,k3-sci-common.yaml#` where they are
> described. I believe this is the correct approach, please advise if
> that is not the case.
>
> .../bindings/remoteproc/ti,k3-m4f-rproc.yaml | 158 ++++++++++++++++++
> 1 file changed, 158 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml
>
> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml
> new file mode 100644
> index 000000000000..1b38df0be2e6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml
> @@ -0,0 +1,158 @@
> +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/remoteproc/ti,k3-m4f-rproc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI K3 M4F processor subsystems
> +
> +maintainers:
> + - Hari Nagalla <[email protected]>
> +
> +description: |
> + Some K3 family SoCs have Arm Cortex M4F cores. AM64x is a SoC in K3
> + family with a M4F core. Typically safety oriented applications may use
> + the M4F core in isolation without an IPC. Where as some industrial and
> + home automation applications, may use the M4F core as a remote processor
> + with IPC communications.
> +
> +$ref: /schemas/arm/keystone/ti,k3-sci-common.yaml#
> +
> +properties:
> + $nodename:
> + pattern: "^m4fss(@.*)?"
Drop. It's not a generic name. Also we do not enforce names in device
schemas.
> +
> + compatible:
> + enum:
> + - ti,am64-m4fss
> +
> + power-domains:
> + description: |
> + Should contain a phandle to a PM domain provider node and an args
> + specifier containing the M4FSS device id value.
Drop description, especially that the args depend on provider, not consumer.
> + maxItems: 1
> +
> + "#address-cells":
> + const: 2
> +
> + "#size-cells":
> + const: 2
> +
> + reg:
> + items:
> + - description: Address and Size of the IRAM internal memory region
Just "IRAM internal memory region"
> + - description: Address and Size of the DRAM internal memory region
> +
> + reg-names:
> + items:
> + - const: iram
> + - const: dram
> +
> + resets:
> + description: |
> + Should contain the phandle to the reset controller node managing the
> + local resets for this device, and a reset specifier.
Drop description.
> + maxItems: 1
> +
> + firmware-name:
> + description: |
> + Should contain the name of the default firmware image
> + file located on the firmware search path
This description is basically duplicating the name... say something
useful or shorten it (e.g. "Should contain" is really redundant). You
also need $ref because we do not have the type defined anywhere.
> +
> + mboxes:
> + description: |
> + OMAP Mailbox specifier denoting the sub-mailbox, to be used for
OMAP?
> + communication with the remote processor. This property should match
> + with the sub-mailbox node used in the firmware image.
> + maxItems: 1
> +
> + memory-region:
> + description: |
> + phandle to the reserved memory nodes to be associated with the
> + remoteproc device. There should be at least two reserved memory nodes
> + defined.
Don't repeat constraints in free form text.
> The reserved memory nodes should be carveout nodes, and
> + should be defined with a "no-map" property as per the bindings in
> + Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
> + minItems: 2
> + maxItems: 8
> + items:
> + - description: region used for dynamic DMA allocations like vrings and
> + vring buffers
> + - description: region reserved for firmware image sections
> + additionalItems: true
And what is the purpose of the rest of reserved nodes?
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - ti,sci
> + - ti,sci-dev-id
> + - ti,sci-proc-ids
> + - resets
> + - firmware-name
> + - mboxes
> + - memory-region
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + mcu_m4fss_dma_memory_region: m4f-dma-memory@9cb00000 {
> + compatible = "shared-dma-pool";
> + reg = <0x00 0x9cb00000 0x00 0x100000>;
> + no-map;
> + };
> +
> + mcu_m4fss_memory_region: m4f-memory@9cc00000 {
> + compatible = "shared-dma-pool";
> + reg = <0x00 0x9cc00000 0x00 0xe00000>;
> + no-map;
> + };
> + };
> +
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + mailbox0_cluster0: mailbox-0 {
> + #mbox-cells = <1>;
> +
> + mbox_m4_0: mbox-m4-0 {
> + ti,mbox-rx = <0 0 0>;
> + ti,mbox-tx = <1 0 0>;
> + };
> + };
Does not look related to this binding... or is it somehow very specific
and needs showing?
> +
> + bus@f0000 {
> + compatible = "simple-bus";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges = <0x00 0x04000000 0x00 0x04000000 0x00 0x01ff1400>;
Why another bus? You already have soc in example, why more nodes?
> +
> + bus@4000000 {
And one more bus?
> + compatible = "simple-bus";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges = <0x00 0x04000000 0x00 0x04000000 0x00 0x01ff1400>;
> +
> + mcu_m4fss: m4fss@5000000 {
Generic node name. Qualcomm uses remoteproc.
> + compatible = "ti,am64-m4fss";
> + reg = <0x00 0x5000000 0x00 0x30000>,
> + <0x00 0x5040000 0x00 0x10000>;
> + reg-names = "iram", "dram";
> + ti,sci = <&dmsc>;
> + ti,sci-dev-id = <9>;
> + ti,sci-proc-ids = <0x18 0xff>;
> + resets = <&k3_reset 9 1>;
> + firmware-name = "am62-mcu-m4f0_0-fw";
> + mboxes = <&mailbox0_cluster0 &mbox_m4_0>;
> + memory-region = <&mcu_m4fss_dma_memory_region>,
> + <&mcu_m4fss_memory_region>;
> + };
> + };
> + };
> + };
Best regards,
Krzysztof
On Fri, 2023-03-03 at 09:06 +0100, Krzysztof Kozlowski wrote:
> On 02/03/2023 18:14, Martyn Welch wrote:
>
> > +
> > + mboxes:
> > + description: |
> > + OMAP Mailbox specifier denoting the sub-mailbox, to be used
> > for
>
> OMAP?
>
This device uses a mailbox compatible with the OMAP Mailbox, as defined
in Documentation/devicetree/bindings/mailbox/ti,omap-mailbox.yaml.
I note that documents title reads "TI OMAP2+ and K3 Mailbox devices".
I'll drop the "OMAP" here.
> > + communication with the remote processor. This property
> > should match
> > + with the sub-mailbox node used in the firmware image.
> > + maxItems: 1
> > +
> > + memory-region:
> > + description: |
> > + phandle to the reserved memory nodes to be associated with
> > the
> > + remoteproc device. There should be at least two reserved
> > memory nodes
> > + defined.
>
> Don't repeat constraints in free form text.
>
> > The reserved memory nodes should be carveout nodes, and
> > + should be defined with a "no-map" property as per the
> > bindings in
> > + Documentation/devicetree/bindings/reserved-memory/reserved-
> > memory.yaml
> > + minItems: 2
> > + maxItems: 8
> > + items:
> > + - description: region used for dynamic DMA allocations like
> > vrings and
> > + vring buffers
> > + - description: region reserved for firmware image sections
> > + additionalItems: true
>
> And what is the purpose of the rest of reserved nodes?
>
Up to 8 regions can be specified with their purpose determined by the
firmware running on the M4F core. The suggestion (and the
implementation in the example firmware) is to use the first 2 regions
as defined above for remoteproc with the others available to be used
for other purposes if necessary. The address translation module used
can cope with up to 8 regions.
>
Martyn
On 07/03/2023 19:26, Martyn Welch wrote:
> On Fri, 2023-03-03 at 09:06 +0100, Krzysztof Kozlowski wrote:
>> On 02/03/2023 18:14, Martyn Welch wrote:
>>
>>> +
>>> + mboxes:
>>> + description: |
>>> + OMAP Mailbox specifier denoting the sub-mailbox, to be used
>>> for
>>
>> OMAP?
>>
>
> This device uses a mailbox compatible with the OMAP Mailbox, as defined
> in Documentation/devicetree/bindings/mailbox/ti,omap-mailbox.yaml.
>
> I note that documents title reads "TI OMAP2+ and K3 Mailbox devices".
> I'll drop the "OMAP" here.
>
>>> + communication with the remote processor. This property
>>> should match
>>> + with the sub-mailbox node used in the firmware image.
>>> + maxItems: 1
>>> +
>>> + memory-region:
>>> + description: |
>>> + phandle to the reserved memory nodes to be associated with
>>> the
>>> + remoteproc device. There should be at least two reserved
>>> memory nodes
>>> + defined.
>>
>> Don't repeat constraints in free form text.
>>
>>> The reserved memory nodes should be carveout nodes, and
>>> + should be defined with a "no-map" property as per the
>>> bindings in
>>> + Documentation/devicetree/bindings/reserved-memory/reserved-
>>> memory.yaml
>>> + minItems: 2
>>> + maxItems: 8
>>> + items:
>>> + - description: region used for dynamic DMA allocations like
>>> vrings and
>>> + vring buffers
>>> + - description: region reserved for firmware image sections
>>> + additionalItems: true
>>
>> And what is the purpose of the rest of reserved nodes?
>>
>
> Up to 8 regions can be specified with their purpose determined by the
> firmware running on the M4F core. The suggestion (and the
> implementation in the example firmware) is to use the first 2 regions
> as defined above for remoteproc with the others available to be used
> for other purposes if necessary. The address translation module used
> can cope with up to 8 regions.
Then mention the rest is just dedicated to the firmware and it's purpose
depends on the firmware.
Best regards,
Krzysztof
Hi Martyn,
Due to the size of this patch my comments will likely span a few days. I will
clearly let you know when I am done reviewing your submission.
On Thu, Mar 02, 2023 at 05:14:48PM +0000, Martyn Welch wrote:
> From: Hari Nagalla <[email protected]>
>
> K3 AM64x SoC has a Cortex M4F subsystem in the MCU voltage domain.
> The remote processor's life cycle management and IPC mechanisms are
> similar across the R5F and M4F cores from remote processor driver
> point of view. However, there are subtle differences in image loading
> and starting the M4F subsystems.
>
> The YAML binding document provides the various node properties to be
> configured by the consumers of the M4F subsystem.
>
> Signed-off-by: Hari Nagalla <[email protected]>
> [Martyn Welch: Amended as per review comments and to pass DT tests]
This isn't needed. Since you are taking over this patchset it is expected you
will make modifications.
> Signed-off-by: Martyn Welch <[email protected]>
> ---
>
> Changes since v1:
> - Spelling corrections
> - Corrected to pass DT checks
>
> Changes since v2:
> - Missed spelling correction to commit message
>
> Note: The only review comment that I don't see directly addressed is the
> lack of description of `ti,sci`, `ti,sci-dev-id` and
> `ti,sci-proc-ids`. A reference has been added to
> `/schemas/arm/keystone/ti,k3-sci-common.yaml#` where they are
> described. I believe this is the correct approach, please advise if
> that is not the case.
>
> .../bindings/remoteproc/ti,k3-m4f-rproc.yaml | 158 ++++++++++++++++++
> 1 file changed, 158 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml
>
> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml
> new file mode 100644
> index 000000000000..1b38df0be2e6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml
> @@ -0,0 +1,158 @@
> +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/remoteproc/ti,k3-m4f-rproc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI K3 M4F processor subsystems
> +
> +maintainers:
> + - Hari Nagalla <[email protected]>
> +
Hari did not finish upstreaming this set. As such I doubt he intents to
maintain it and I suppose it is the same for you since you didn't add your name
either. You can add my name there.
> +description: |
> + Some K3 family SoCs have Arm Cortex M4F cores. AM64x is a SoC in K3
> + family with a M4F core. Typically safety oriented applications may use
> + the M4F core in isolation without an IPC. Where as some industrial and
> + home automation applications, may use the M4F core as a remote processor
> + with IPC communications.
> +
> +$ref: /schemas/arm/keystone/ti,k3-sci-common.yaml#
> +
> +properties:
> + $nodename:
> + pattern: "^m4fss(@.*)?"
> +
> + compatible:
> + enum:
> + - ti,am64-m4fss
> +
> + power-domains:
> + description: |
> + Should contain a phandle to a PM domain provider node and an args
> + specifier containing the M4FSS device id value.
> + maxItems: 1
> +
> + "#address-cells":
> + const: 2
> +
> + "#size-cells":
> + const: 2
> +
> + reg:
> + items:
> + - description: Address and Size of the IRAM internal memory region
> + - description: Address and Size of the DRAM internal memory region
> +
> + reg-names:
> + items:
> + - const: iram
> + - const: dram
> +
> + resets:
> + description: |
> + Should contain the phandle to the reset controller node managing the
> + local resets for this device, and a reset specifier.
> + maxItems: 1
> +
> + firmware-name:
> + description: |
> + Should contain the name of the default firmware image
> + file located on the firmware search path
> +
> + mboxes:
> + description: |
> + OMAP Mailbox specifier denoting the sub-mailbox, to be used for
> + communication with the remote processor. This property should match
> + with the sub-mailbox node used in the firmware image.
> + maxItems: 1
> +
> + memory-region:
> + description: |
> + phandle to the reserved memory nodes to be associated with the
> + remoteproc device. There should be at least two reserved memory nodes
> + defined. The reserved memory nodes should be carveout nodes, and
> + should be defined with a "no-map" property as per the bindings in
> + Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
> + minItems: 2
> + maxItems: 8
> + items:
> + - description: region used for dynamic DMA allocations like vrings and
> + vring buffers
> + - description: region reserved for firmware image sections
> + additionalItems: true
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - ti,sci
> + - ti,sci-dev-id
> + - ti,sci-proc-ids
> + - resets
> + - firmware-name
> + - mboxes
The 'mboxes' property is marked as required but the description section above
clearly state the M4F can operate without IPC.
> + - memory-region
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + mcu_m4fss_dma_memory_region: m4f-dma-memory@9cb00000 {
> + compatible = "shared-dma-pool";
> + reg = <0x00 0x9cb00000 0x00 0x100000>;
> + no-map;
> + };
> +
> + mcu_m4fss_memory_region: m4f-memory@9cc00000 {
> + compatible = "shared-dma-pool";
> + reg = <0x00 0x9cc00000 0x00 0xe00000>;
> + no-map;
> + };
> + };
> +
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + mailbox0_cluster0: mailbox-0 {
> + #mbox-cells = <1>;
> +
> + mbox_m4_0: mbox-m4-0 {
> + ti,mbox-rx = <0 0 0>;
> + ti,mbox-tx = <1 0 0>;
> + };
> + };
> +
> + bus@f0000 {
> + compatible = "simple-bus";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges = <0x00 0x04000000 0x00 0x04000000 0x00 0x01ff1400>;
> +
> + bus@4000000 {
> + compatible = "simple-bus";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges = <0x00 0x04000000 0x00 0x04000000 0x00 0x01ff1400>;
> +
> + mcu_m4fss: m4fss@5000000 {
> + compatible = "ti,am64-m4fss";
> + reg = <0x00 0x5000000 0x00 0x30000>,
> + <0x00 0x5040000 0x00 0x10000>;
> + reg-names = "iram", "dram";
> + ti,sci = <&dmsc>;
> + ti,sci-dev-id = <9>;
> + ti,sci-proc-ids = <0x18 0xff>;
> + resets = <&k3_reset 9 1>;
> + firmware-name = "am62-mcu-m4f0_0-fw";
> + mboxes = <&mailbox0_cluster0 &mbox_m4_0>;
This doesn't look right since mbox_m4_0 is defined within mailbox0_cluster0.
> + memory-region = <&mcu_m4fss_dma_memory_region>,
> + <&mcu_m4fss_memory_region>;
> + };
> + };
> + };
> + };
> --
> 2.39.1
>
On 3/8/23 14:58, Mathieu Poirier wrote:
>> +required:
>> + - compatible
>> + - reg
>> + - reg-names
>> + - ti,sci
>> + - ti,sci-dev-id
>> + - ti,sci-proc-ids
>> + - resets
>> + - firmware-name
>> + - mboxes
> The 'mboxes' property is marked as required but the description section above
> clearly state the M4F can operate without IPC.
>
Well, when the M4F is used as a safety processor it is typically booted
from SBL/u-boot and may isolate the MCU domain from main domain/A53 to
function in higher safety level. In these scenarios there is no remote
proc handling of M4F life cycle management (LCM) and IPC. But, on the
other hand, when the M4F is used as a non safety processor its LCM is
handled by remote proc(main domain) and mailboxes for IPC are required.
On Thu, Mar 09, 2023 at 05:18:01PM -0600, Hari Nagalla wrote:
> On 3/8/23 14:58, Mathieu Poirier wrote:
> > > +required:
> > > + - compatible
> > > + - reg
> > > + - reg-names
> > > + - ti,sci
> > > + - ti,sci-dev-id
> > > + - ti,sci-proc-ids
> > > + - resets
> > > + - firmware-name
> > > + - mboxes
> > The 'mboxes' property is marked as required but the description section above
> > clearly state the M4F can operate without IPC.
> >
> Well, when the M4F is used as a safety processor it is typically booted from
> SBL/u-boot and may isolate the MCU domain from main domain/A53 to function
> in higher safety level. In these scenarios there is no remote proc handling
> of M4F life cycle management (LCM) and IPC. But, on the other hand, when the
> M4F is used as a non safety processor its LCM is handled by remote proc(main
> domain) and mailboxes for IPC are required.
Well, what you wrote above is pretty much explained verbatim in the
"description" section of the bindings. Mailboxes are optional and as such
should not be found under the "required" section.
On Fri, 2023-03-10 at 08:41 -0700, Mathieu Poirier wrote:
> On Thu, Mar 09, 2023 at 05:18:01PM -0600, Hari Nagalla wrote:
> > On 3/8/23 14:58, Mathieu Poirier wrote:
> > > > +required:
> > > > + - compatible
> > > > + - reg
> > > > + - reg-names
> > > > + - ti,sci
> > > > + - ti,sci-dev-id
> > > > + - ti,sci-proc-ids
> > > > + - resets
> > > > + - firmware-name
> > > > + - mboxes
> > > The 'mboxes' property is marked as required but the description
> > > section above
> > > clearly state the M4F can operate without IPC.
> > >
> > Well, when the M4F is used as a safety processor it is typically
> > booted from
> > SBL/u-boot and may isolate the MCU domain from main domain/A53 to
> > function
> > in higher safety level. In these scenarios there is no remote proc
> > handling
> > of M4F life cycle management (LCM) and IPC. But, on the other hand,
> > when the
> > M4F is used as a non safety processor its LCM is handled by remote
> > proc(main
> > domain) and mailboxes for IPC are required.
>
> Well, what you wrote above is pretty much explained verbatim in the
> "description" section of the bindings. Mailboxes are optional and as
> such
> should not be found under the "required" section.
>
Which means the memory regions are also optional as in the isolated
case they're be no communications with the main domain.
Martyn
On Wed, 2023-03-08 at 13:58 -0700, Mathieu Poirier wrote:
> On Thu, Mar 02, 2023 at 05:14:48PM +0000, Martyn Welch wrote:
>
> > +examples:
> > + - |
> > + reserved-memory {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > +
> > + mcu_m4fss_dma_memory_region: m4f-dma-memory@9cb00000 {
> > + compatible = "shared-dma-pool";
> > + reg = <0x00 0x9cb00000 0x00 0x100000>;
> > + no-map;
> > + };
> > +
> > + mcu_m4fss_memory_region: m4f-memory@9cc00000 {
> > + compatible = "shared-dma-pool";
> > + reg = <0x00 0x9cc00000 0x00 0xe00000>;
> > + no-map;
> > + };
> > + };
> > +
> > + soc {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > +
> > + mailbox0_cluster0: mailbox-0 {
> > + #mbox-cells = <1>;
> > +
> > + mbox_m4_0: mbox-m4-0 {
> > + ti,mbox-rx = <0 0 0>;
> > + ti,mbox-tx = <1 0 0>;
> > + };
> > + };
> > +
> > + bus@f0000 {
> > + compatible = "simple-bus";
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > + ranges = <0x00 0x04000000 0x00 0x04000000 0x00
> > 0x01ff1400>;
> > +
> > + bus@4000000 {
> > + compatible = "simple-bus";
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > + ranges = <0x00 0x04000000 0x00 0x04000000 0x00
> > 0x01ff1400>;
> > +
> > + mcu_m4fss: m4fss@5000000 {
> > + compatible = "ti,am64-m4fss";
> > + reg = <0x00 0x5000000 0x00 0x30000>,
> > + <0x00 0x5040000 0x00 0x10000>;
> > + reg-names = "iram", "dram";
> > + ti,sci = <&dmsc>;
> > + ti,sci-dev-id = <9>;
> > + ti,sci-proc-ids = <0x18 0xff>;
> > + resets = <&k3_reset 9 1>;
> > + firmware-name = "am62-mcu-m4f0_0-fw";
> > + mboxes = <&mailbox0_cluster0 &mbox_m4_0>;
>
> This doesn't look right since mbox_m4_0 is defined within
> mailbox0_cluster0.
>
Looking at other users of the omap mailboxes (and not wanting to spend
ages trawling back through the mailing list archives to try and work
out why it's specified the way it is) it seems that this is the way
that these mailboxes are specified.
For instance in `arch/arm/boot/dts/omap4-l4.dtsi`, the mailbox is
defined:
mailbox: mailbox@0 {
compatible = "ti,omap4-mailbox";
reg = <0x0 0x200>;
interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
#mbox-cells = <1>;
ti,mbox-num-users = <3>;
ti,mbox-num-fifos = <8>;
mbox_ipu: mbox-ipu {
ti,mbox-tx = <0 0 0>;
ti,mbox-rx = <1 0 0>;
};
mbox_dsp: mbox-dsp {
ti,mbox-tx = <3 0 0>;
ti,mbox-rx = <2 0 0>;
};
};
It's users in `arch/arm/boot/dts/omap4.dtsi`:
dsp: dsp {
...
mboxes = <&mailbox &mbox_dsp>;
...
};
ipu: ipu@55020000 {
...
mboxes = <&mailbox &mbox_ipu>;
...
};
> > + memory-region =
> > <&mcu_m4fss_dma_memory_region>,
> > + <&mcu_m4fss_memory_region>;
> > + };
> > + };
> > + };
> > + };
> > --
> > 2.39.1
> >