Re: [PATCH] dt-bindings: stm32: convert mlahb to json-schema

From: Rob Herring
Date: Mon Dec 16 2019 - 12:10:33 EST

On Mon, Dec 16, 2019 at 2:44 AM Arnaud POULIQUEN
<arnaud.pouliquen@xxxxxx> wrote:
>
> Hello Rob,
>
> On 12/13/19 10:39 PM, Rob Herring wrote:
> > On Thu, Nov 28, 2019 at 04:46:03PM +0100, Arnaud Pouliquen wrote:
> >> Convert the ML-AHB bus bindings to DT schema format using json-schema
> >>
> >> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@xxxxxx>
> >> ---
> >> Notice that this patch requests an update of the simple-bus schema to add
> >> the support of the "dma-ranges" property.
> >> A Pull request has been sent in parallel to the dt-schema github repo:
> >> https://github.com/devicetree-org/dt-schema/pull/30
> >>
> >> To remind the topic around the use of "dma-ranges" please
> >> refer to following discussion: https://lkml.org/lkml/2019/4/3/1261
> >> ---
> >> .../devicetree/bindings/arm/stm32/mlahb.txt | 37 ----------
> >> .../bindings/arm/stm32/st,mlahb.yaml | 69 +++++++++++++++++++
> >> 2 files changed, 69 insertions(+), 37 deletions(-)
> >> delete mode 100644 Documentation/devicetree/bindings/arm/stm32/mlahb.txt
> >> create mode 100644 Documentation/devicetree/bindings/arm/stm32/st,mlahb.yaml
> >>
> >> diff --git a/Documentation/devicetree/bindings/arm/stm32/mlahb.txt b/Documentation/devicetree/bindings/arm/stm32/mlahb.txt
> >> deleted file mode 100644
> >> index 25307aa1eb9b..000000000000
> >> --- a/Documentation/devicetree/bindings/arm/stm32/mlahb.txt
> >> +++ /dev/null
> >> @@ -1,37 +0,0 @@
> >> -ML-AHB interconnect bindings
> >> -
> >> -These bindings describe the STM32 SoCs ML-AHB interconnect bus which connects
> >> -a Cortex-M subsystem with dedicated memories.
> >> -The MCU SRAM and RETRAM memory parts can be accessed through different addresses
> >> -(see "RAM aliases" in [1]) using different buses (see [2]) : balancing the
> >> -Cortex-M firmware accesses among those ports allows to tune the system
> >> -performance.
> >> -
> >> -[1]: https://www.st.com/resource/en/reference_manual/dm00327659.pdf
> >> -[2]: https://wiki.st.com/stm32mpu/wiki/STM32MP15_RAM_mapping
> >> -
> >> -Required properties:
> >> -- compatible: should be "simple-bus"
> >> -- dma-ranges: describes memory addresses translation between the local CPU and
> >> - the remote Cortex-M processor. Each memory region, is declared with
> >> - 3 parameters:
> >> - - param 1: device base address (Cortex-M processor address)
> >> - - param 2: physical base address (local CPU address)
> >> - - param 3: size of the memory region.
> >> -
> >> -The Cortex-M remote processor accessed via the mlahb interconnect is described
> >> -by a child node.
> >> -
> >> -Example:
> >> -mlahb {
> >> - compatible = "simple-bus";
> >> - #address-cells = <1>;
> >> - #size-cells = <1>;
> >> - dma-ranges = <0x00000000 0x38000000 0x10000>,
> >> - <0x10000000 0x10000000 0x60000>,
> >> - <0x30000000 0x30000000 0x60000>;
> >> -
> >> - m4_rproc: m4@10000000 {
> >> - ...
> >> - };
> >> -};
> >> diff --git a/Documentation/devicetree/bindings/arm/stm32/st,mlahb.yaml b/Documentation/devicetree/bindings/arm/stm32/st,mlahb.yaml
> >> new file mode 100644
> >> index 000000000000..8ad3f7c7f9ab
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/arm/stm32/st,mlahb.yaml
> >> @@ -0,0 +1,69 @@
> >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> >> +%YAML 1.2
> >> +---
> >> +$id: "http://devicetree.org/schemas/arm/stm32/st,mlahb.yaml#";
> >> +$schema: "http://devicetree.org/meta-schemas/core.yaml#";
> >> +
> >> +title: STMicroelectronics STM32 ML-AHB interconnect bindings
> >> +
> >> +maintainers:
> >> + - Fabien Dessenne <fabien.dessenne@xxxxxx>
> >> + - Arnaud Pouliquen <arnaud.pouliquen@xxxxxx>
> >> +
> >> +description: |
> >> + These bindings describe the STM32 SoCs ML-AHB interconnect bus which connects
> >> + a Cortex-M subsystem with dedicated memories. The MCU SRAM and RETRAM memory
> >> + parts can be accessed through different addresses (see "RAM aliases" in [1])
> >> + using different buses (see [2]): balancing the Cortex-M firmware accesses
> >> + among those ports allows to tune the system performance.
> >> + [1]: https://www.st.com/resource/en/reference_manual/dm00327659.pdf
> >> + [2]: https://wiki.st.com/stm32mpu/wiki/STM32MP15_RAM_mapping
> >> +
> >> +allOf:
> >> + - $ref: /schemas/simple-bus.yaml#
> >> +
> >> +properties:
> >> + compatible:
> >> + contains:
> >> + enum:
> >> + - st,mlahb
> >> +
> >> + dma-ranges:
> >> + description: |
> >> + Describe memory addresses translation between the local CPU and the
> >> + remote Cortex-M processor. Each memory region, is declared with
> >> + 3 parameters:
> >> + - param 1: device base address (Cortex-M processor address)
> >> + - param 2: physical base address (local CPU address)
> >> + - param 3: size of the memory region.
> >> + maxItems: 3
> >> +
> >> + '#address-cells':
> >> + const: 1
> >> +
> >> + '#size-cells':
> >> + const: 1
> >> +
> >> +required:
> >> + - compatible
> >> + - '#address-cells'
> >> + - '#size-cells'
> >> + - dma-ranges
> >> +
> >> +examples:
> >> + - |
> >> + mlahb: ahb {
> >> + compatible = "st,mlahb", "simple-bus";
> >> + #address-cells = <1>;
> >> + #size-cells = <1>;
> >> + reg = <0x10000000 0x40000>;
> >> + dma-ranges = <0x00000000 0x38000000 0x10000>,
> >> + <0x10000000 0x10000000 0x60000>,
> >> + <0x30000000 0x30000000 0x60000>;
> >> +
> >
> > Fails to build:
> >
> > builds/robherring/linux-dt-review/Documentation/devicetree/bindings/arm/stm32/st,mlahb.example.dt.yaml:
> > ahb: 'ranges' is a required property
> >
> > Run 'make dt_binding_check'
>
> Yes, that why i posted in parallel the PR on the tool:
> https://github.com/devicetree-org/dt-schema/pull/30.
>
> I don't know if you saw my answer on this post so i copy/paste it here:
>
> How to describe the stm32mp1 coprocessor mapping and translations based on this "ranges" requirement?

Minimally, you just need to add an empty ranges. Otherwise, the
0x10000000 in reg in the child node is not considered a MMIO address.

> On stm32mp1 we have 2 RAM memories accessible by the Cortex-A7 running Linux and used by the Cortex-M4 coprocessor to run its code. Each Cortex has a specific mapping of the memories. Some memory translations are needed by:
>
> - rproc driver to load the Cortex-M4 firmware in RAMs
> - rpmsg & virtio frameworks for shared memory
>
> Here is the memory mapping:
>
> - RETRAM (64 kB);
> - Cortex-A7 @ : 0x38000000
> - Cortex-M4 @: 0x00000000
> - MCUSRAM (up to 384 kB)
> - Cortex-A7 @ : 0x30000000 or 0x10000000 ( 2 addresses for the same memory)
> - Cortex-M4 @: 0x30000000 or 0x10000000 ( 2 addresses for the same memory)
> =>addresses used on both side depend on the MCURAM access optimization
>
> Today our upstreamed solution is based on dma-range only (https://lkml.org/lkml/2019/4/3/1261)
>
> What about adding an "unused" ranges property in DT to match the requirement?
>
> mlahb {
> compatible = "simple-bus";
> #address-cells = <1>;
> #size-cells = <1>;
> ranges = <0 0x38000000 0x10000>,
> <1 0x10000000 0x60000>,
> <2 0x30000000 0x60000>;

This is not valid with child addresses of 0, 1, and 2. 'ranges' is
purely about the view from the A7 to the M4. If you want to point 2
parent addresses to the same child region, you could do:

ranges = <0x38000000 0x38000000 0x10000>,
<0x10000000 0x10000000 0x60000>,
<0x10000000 0x30000000 0x60000>;

Though I'm not sure what the OS address translation code will do in
this case. Probably just ignore the last entry.

> dma-ranges = <0x00000000 0x38000000 0x10000>,
> <0x10000000 0x10000000 0x60000>,
> <0x30000000 0x30000000 0x60000>;
>
> m4_rproc: m4@10000000 {
> ---
> };
> };
>
> Thanks in advance for your feedback.
> Regards,
> Arnaud
>
> >
> >> + m4_rproc: m4@10000000 {
> >> + reg = <0x10000000 0x40000>;
> >> + };
> >> + };
> >> +
> >> +...
> >> --
> >> 2.17.1
> >>