[PATCH v3 1/5] dt-bindings: bus: add documentation for the IMX AIPSTZ bridge

[Marco Felsch]

On 25-03-26, Marco Felsch wrote:
> Hi Laurentiu,
> 
> On 25-03-24, Rob Herring wrote:
> > On Mon, Mar 24, 2025 at 12:25:52PM -0400, Laurentiu Mihalcea wrote:
> > > From: Laurentiu Mihalcea <laurentiu.mihalcea at nxp.com>
> > > 
> > > Add documentation for IMX AIPSTZ bridge.
> > > 
> > > Co-developed-by: Daniel Baluta <daniel.baluta at nxp.com>
> > > Signed-off-by: Daniel Baluta <daniel.baluta at nxp.com>
> > > Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea at nxp.com>
> > > ---
> > >  .../bindings/bus/fsl,imx8mp-aipstz.yaml       | 107 ++++++++++++++++++
> > >  1 file changed, 107 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/bus/fsl,imx8mp-aipstz.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/bus/fsl,imx8mp-aipstz.yaml b/Documentation/devicetree/bindings/bus/fsl,imx8mp-aipstz.yaml
> > > new file mode 100644
> > > index 000000000000..c0427dfcdaca
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/bus/fsl,imx8mp-aipstz.yaml
> > > @@ -0,0 +1,107 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/bus/fsl,imx8mp-aipstz.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Secure AHB to IP Slave bus (AIPSTZ) bridge
> > > +
> > > +description:
> > > +  The secure AIPS bridge (AIPSTZ) acts as a bridge for AHB masters
> > > +  issuing transactions to IP Slave peripherals. Additionally, this module
> > > +  offers access control configurations meant to restrict which peripherals
> > > +  a master can access.
> > 
> > Wrap at 80 chars.
> > 
> > > +
> > > +maintainers:
> > > +  - Laurentiu Mihalcea <laurentiu.mihalcea at nxp.com>
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: fsl,imx8mp-aipstz
> > > +
> > > +  reg:
> > > +    maxItems: 2
> > > +
> > > +  reg-names:
> > > +    items:
> > > +      - const: bus
> > > +      - const: ac
> > > +
> > > +  power-domains:
> > > +    maxItems: 1
> > > +
> > > +  "#address-cells":
> > > +    const: 1
> > > +
> > > +  "#size-cells":
> > > +    const: 1
> > > +
> > > +  "#access-controller-cells":
> > > +    const: 0
> > 
> > With 0 cells, how do you identify which device it is?
> > 
> > > +
> > > +  ranges: true
> > > +
> > > +# borrowed from simple-bus.yaml, no additional requirements for children
> > > +patternProperties:
> > > +  "@(0|[1-9a-f][0-9a-f]*)$":
> > > +    type: object
> > > +    additionalProperties: true
> > > +    properties:
> > > +      reg:
> > > +        items:
> > > +          minItems: 2
> > > +          maxItems: 4
> > > +        minItems: 1
> > > +        maxItems: 1024
> > > +      ranges:
> > > +        oneOf:
> > > +          - items:
> > > +              minItems: 3
> > > +              maxItems: 7
> > > +            minItems: 1
> > > +            maxItems: 1024
> > > +          - $ref: /schemas/types.yaml#/definitions/flag
> > > +    anyOf:
> > > +      - required:
> > > +          - reg
> > > +      - required:
> > > +          - ranges
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - reg-names
> > > +  - power-domains
> > > +  - "#address-cells"
> > > +  - "#size-cells"
> > > +  - "#access-controller-cells"
> > > +  - ranges
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    #include <dt-bindings/clock/imx8mp-clock.h>
> > > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > +
> > > +    bus at 30c00000 {
> > > +        compatible = "fsl,imx8mp-aipstz";
> > > +        reg = <0x30c00000 0x400000>, <0x30df0000 0x10000>;
> > 
> > It doesn't look like you have any registers in the 1st entry, but they 
> > are child devices? Then you should use ranges and drop it here:
> > 
> > ranges = <0x0 0x30c00000 0x400000>;
> > 
> > 
> > > +        reg-names = "bus", "ac";
> 
> Thanks for picking up my suggestion :) IMHO it does look more logical
> now. I wasn't aware of the 'ranges' property else I would have suggested
> you to use this property instead of having two regs, sorry. Once you
> changed it to ranges we can drop the 'reg-names' as well since you only
> need to supply the 'ac' register space.
> 
> Regards,
>   Marco
> 
> > > +        power-domains = <&pgc_audio>;
> > > +        #address-cells = <1>;
> > > +        #size-cells = <1>;
> > > +        #access-controller-cells = <0>;
> > > +        ranges;

I didn't noticed that we already do have the ranges 1:1 mapping, sorry!

@Rob
A "ranges = <0x0 0x30c00000 0x400000>;" would make writing/syncing the
.dtsi harder since NXP decided to use use global addresses in their
technical reference manual.

@Laurentiu
Could you please add a "ranges = <0x30c00000 0x30c00000 0x400000>;" for
the bus? Of course it is still a 1:1 mapping but limits the bus size
which can be helpful if someone add a device on the wrong bus.

Sorry for my previous suggestion on your V2 which seems more reasonable
now.

Regards,
  Marco

> > > +        dma-controller at 30e00000 {
> > > +            compatible = "fsl,imx8mp-sdma", "fsl,imx8mq-sdma";
> > > +            reg = <0x30e00000 0x10000>;
> > > +            #dma-cells = <3>;
> > > +            clocks = <&audio_blk_ctrl IMX8MP_CLK_AUDIOMIX_SDMA3_ROOT>,
> > > +                     <&clk IMX8MP_CLK_AUDIO_ROOT>;
> > > +            clock-names = "ipg", "ahb";
> > > +            interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
> > > +            fsl,sdma-ram-script-name = "imx/sdma/sdma-imx7d.bin";
> > 
> > No 'access-controllers' here?
> > 
> > > +        };
> > > +    };
> > > -- 
> > > 2.34.1
> > > 
> > 
> 
>