[PATCH V4 1/5] dt-bindings: mailbox: allow mbox-cells to be equal to 0

[PATCH V4 1/5] dt-bindings: mailbox: allow mbox-cells to be equal to 0

[Dong Aisheng]

Mailbox devices may have only one channel which means the mbox-cells
at least 1 does not make sense for this type devices. Let's remove
that limitation to allow the mbox-cells to be equal to 0.

Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
ChangeLog:
 * New patch introduced in v3 series
 * Actually there're already users in kernel with mbox-cells set to 0.
   See:
   arch/arm/boot/dts/bcm283x.dtsi:145: #mbox-cells = <0>;
---
 Documentation/devicetree/bindings/mailbox/mailbox.txt | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mailbox/mailbox.txt b/Documentation/devicetree/bindings/mailbox/mailbox.txt
index af8ecee..c2fcd05 100644
--- a/Documentation/devicetree/bindings/mailbox/mailbox.txt
+++ b/Documentation/devicetree/bindings/mailbox/mailbox.txt
@@ -6,8 +6,7 @@ assign appropriate mailbox channel to client drivers.
 * Mailbox Controller
 
 Required property:
-- #mbox-cells: Must be at least 1. Number of cells in a mailbox
-		specifier.
+- #mbox-cells: Number of cells in a mailbox specifier.
 
 Example:
 	mailbox: mailbox {
-- 
2.7.4

[PATCH V4 2/5] dt-bindings: arm: fsl: add mu binding doc

[Dong Aisheng]

The Messaging Unit module enables two processors within
the SoC to communicate and coordinate by passing messages
(e.g. data, status and control) through the MU interface.

Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: devicetree@vger.kernel.org
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
v3->v4:
 * move into Documentation/devicetree/bindings/mailbox/fsl,mu.txt
v2->v3:
 * change to mailbox binding
   Currently mbox-cells has to be 0 for SCU MU.
   Generic MU Mailbox support binding could be extended later.
v1->v2:
 * typo fixes
 * remove status property
 * remove imx6&7 compatible string which may be added later for
   the generic mailbox binding
---
 .../devicetree/bindings/mailbox/fsl,mu.txt         | 34 ++++++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/fsl,mu.txt

diff --git a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt
new file mode 100644
index 0000000..90e4905
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt
@@ -0,0 +1,34 @@
+NXP i.MX Messaging Unit (MU)
+--------------------------------------------------------------------
+
+The Messaging Unit module enables two processors within the SoC to
+communicate and coordinate by passing messages (e.g. data, status
+and control) through the MU interface. The MU also provides the ability
+for one processor to signal the other processor using interrupts.
+
+Because the MU manages the messaging between processors, the MU uses
+different clocks (from each side of the different peripheral buses).
+Therefore, the MU must synchronize the accesses from one side to the
+other. The MU accomplishes synchronization using two sets of matching
+registers (Processor A-facing, Processor B-facing).
+
+Messaging Unit Device Node:
+=============================
+
+Required properties:
+-------------------
+- compatible :	should be "fsl,<chip>-mu", the supported chips include
+		imx8qxp, imx8qm.
+- reg :		Should contain the registers location and length
+- interrupts :	Interrupt number. The interrupt specifier format depends
+		on the interrupt controller parent.
+- #mbox-cells:  Must be 0. Number of cells in a mailbox
+
+Examples:
+--------
+lsio_mu0: mailbox@5d1b0000 {
+	compatible = "fsl,imx8qxp-mu";
+	reg = <0x0 0x5d1b0000 0x0 0x10000>;
+	interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>;
+	#mbox-cells = <0>;
+};
-- 
2.7.4

[PATCH V4 4/5] dt-bindings: arm: fsl: add scu binding doc

[Dong Aisheng]

The System Controller Firmware (SCFW) is a low-level system function
which runs on a dedicated Cortex-M core to provide power, clock, and
resource management. It exists on some i.MX8 processors. e.g. i.MX8QM
(QM, QP), and i.MX8QX (QXP, DX).

Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: devicetree@vger.kernel.org
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
v3->v4:
 * fully change to mailbox binding
 * add child node description
v2->v3:
 * update a bit to mailbox binding
v1->v2:
 * remove status
 * changed to mu1
---
 .../devicetree/bindings/arm/freescale/fsl,scu.txt  | 65 ++++++++++++++++++++++
 1 file changed, 65 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt

diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
new file mode 100644
index 0000000..11e732a
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
@@ -0,0 +1,65 @@
+NXP i.MX System Controller Firmware (SCFW)
+--------------------------------------------------------------------
+
+The System Controller Firmware (SCFW) is a low-level system function
+which runs on a dedicated Cortex-M core to provide power, clock, and
+resource management. It exists on some i.MX8 processors. e.g. i.MX8QM
+(QM, QP), and i.MX8QX (QXP, DX).
+
+The AP communicates with the SC using a multi-ported MU module found
+in the LSIO subsystem. The current definition of this MU module provides
+5 remote AP connections to the SC to support up to 5 execution environments
+(TZ, HV, standard Linux, etc.). The SC side of this MU module interfaces
+with the LSIO DSC IP bus. The SC firmware will communicate with this MU
+using the MSI bus.
+
+System Controller Device Node:
+=============================
+
+Required properties:
+-------------------
+- compatible: should be "fsl,imx-scu"
+- mboxes: List of phandle of MU mailbox. Should be one of LSIO
+	  MU0~M4 for imx8qxp and imx8qm. Users need to make
+	  sure not use the one which is conflict with other
+	  execution environments. e.g. ATF.
+
+Examples:
+--------
+lsio_mu1: mailbox@5d1c0000 {
+	compatible = "fsl,imx8qxp-mu";
+	reg = <0x0 0x5d1c0000 0x0 0x10000>;
+	interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>;
+	#mbox-cells = <0>;
+};
+
+scu {
+	compatible = "fsl,imx-scu";
+	mboxes = <&lsio_mu1>;
+};
+
+
+i.MX SCU Client Device Node:
+=========================
+
+Client nodes are maintained as children of the relevant IMX-SCU device node.
+
+Example (imx8qxp):
+-------------
+scu {
+	compatible = "fsl,imx-scu";
+	...
+
+	clk: clk {
+		...
+	};
+
+	iomuxc: iomuxc {
+		...
+	};
+
+	imx8qx-pm {
+		...
+	}
+	...
+};
-- 
2.7.4

Re: [PATCH V4 4/5] dt-bindings: arm: fsl: add scu binding doc

[Rob Herring]

On Sun, Jul 08, 2018 at 10:56:56PM +0800, Dong Aisheng wrote:
> The System Controller Firmware (SCFW) is a low-level system function
> which runs on a dedicated Cortex-M core to provide power, clock, and
> resource management. It exists on some i.MX8 processors. e.g. i.MX8QM
> (QM, QP), and i.MX8QX (QXP, DX).
> 
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---
> v3->v4:
>  * fully change to mailbox binding
>  * add child node description
> v2->v3:
>  * update a bit to mailbox binding
> v1->v2:
>  * remove status
>  * changed to mu1
> ---
>  .../devicetree/bindings/arm/freescale/fsl,scu.txt  | 65 ++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> new file mode 100644
> index 0000000..11e732a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> @@ -0,0 +1,65 @@
> +NXP i.MX System Controller Firmware (SCFW)
> +--------------------------------------------------------------------
> +
> +The System Controller Firmware (SCFW) is a low-level system function
> +which runs on a dedicated Cortex-M core to provide power, clock, and
> +resource management. It exists on some i.MX8 processors. e.g. i.MX8QM
> +(QM, QP), and i.MX8QX (QXP, DX).
> +
> +The AP communicates with the SC using a multi-ported MU module found
> +in the LSIO subsystem. The current definition of this MU module provides
> +5 remote AP connections to the SC to support up to 5 execution environments
> +(TZ, HV, standard Linux, etc.). The SC side of this MU module interfaces
> +with the LSIO DSC IP bus. The SC firmware will communicate with this MU
> +using the MSI bus.
> +
> +System Controller Device Node:
> +=============================

Need to specify this is a child of /firmware node.

> +
> +Required properties:
> +-------------------
> +- compatible: should be "fsl,imx-scu"
> +- mboxes: List of phandle of MU mailbox. Should be one of LSIO
> +	  MU0~M4 for imx8qxp and imx8qm. Users need to make
> +	  sure not use the one which is conflict with other
> +	  execution environments. e.g. ATF.
> +
> +Examples:
> +--------
> +lsio_mu1: mailbox@5d1c0000 {
> +	compatible = "fsl,imx8qxp-mu";
> +	reg = <0x0 0x5d1c0000 0x0 0x10000>;
> +	interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>;
> +	#mbox-cells = <0>;
> +};
> +
> +scu {
> +	compatible = "fsl,imx-scu";
> +	mboxes = <&lsio_mu1>;
> +};
> +
> +
> +i.MX SCU Client Device Node:
> +=========================
> +
> +Client nodes are maintained as children of the relevant IMX-SCU device node.
> +
> +Example (imx8qxp):
> +-------------
> +scu {
> +	compatible = "fsl,imx-scu";
> +	...
> +
> +	clk: clk {
> +		...
> +	};
> +
> +	iomuxc: iomuxc {
> +		...
> +	};
> +
> +	imx8qx-pm {
> +		...
> +	}
> +	...

If you plan to have child nodes, you need to document them now. However, 
other than pinctrl, you probably don't need child nodes. The parent node 
can be a clock and power-domain provider.

Rob

Re: [PATCH V4 2/5] dt-bindings: arm: fsl: add mu binding doc

[Rob Herring]

On Sun, Jul 08, 2018 at 10:56:54PM +0800, Dong Aisheng wrote:
> The Messaging Unit module enables two processors within
> the SoC to communicate and coordinate by passing messages
> (e.g. data, status and control) through the MU interface.
> 
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---
> v3->v4:
>  * move into Documentation/devicetree/bindings/mailbox/fsl,mu.txt
> v2->v3:
>  * change to mailbox binding
>    Currently mbox-cells has to be 0 for SCU MU.
>    Generic MU Mailbox support binding could be extended later.
> v1->v2:
>  * typo fixes
>  * remove status property
>  * remove imx6&7 compatible string which may be added later for
>    the generic mailbox binding
> ---
>  .../devicetree/bindings/mailbox/fsl,mu.txt         | 34 ++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/fsl,mu.txt

Reviewed-by: Rob Herring <robh@kernel.org>

RE: [PATCH V4 4/5] dt-bindings: arm: fsl: add scu binding doc

[A.s. Dong]

> -----Original Message-----
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: Wednesday, July 11, 2018 11:09 PM
> To: A.s. Dong <aisheng.dong@nxp.com>
> Cc: linux-arm-kernel@lists.infradead.org; dongas86@gmail.com;
> kernel@pengutronix.de; shawnguo@kernel.org; Fabio Estevam
> <fabio.estevam@nxp.com>; dl-linux-imx <linux-imx@nxp.com>; Mark
> Rutland <mark.rutland@arm.com>; devicetree@vger.kernel.org
> Subject: Re: [PATCH V4 4/5] dt-bindings: arm: fsl: add scu binding doc
> 
> On Sun, Jul 08, 2018 at 10:56:56PM +0800, Dong Aisheng wrote:
> > The System Controller Firmware (SCFW) is a low-level system function
> > which runs on a dedicated Cortex-M core to provide power, clock, and
> > resource management. It exists on some i.MX8 processors. e.g. i.MX8QM
> > (QM, QP), and i.MX8QX (QXP, DX).
> >
> > Cc: Shawn Guo <shawnguo@kernel.org>
> > Cc: Sascha Hauer <kernel@pengutronix.de>
> > Cc: Fabio Estevam <fabio.estevam@nxp.com>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> > ---
> > v3->v4:
> >  * fully change to mailbox binding
> >  * add child node description
> > v2->v3:
> >  * update a bit to mailbox binding
> > v1->v2:
> >  * remove status
> >  * changed to mu1
> > ---
> >  .../devicetree/bindings/arm/freescale/fsl,scu.txt  | 65
> > ++++++++++++++++++++++
> >  1 file changed, 65 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> >
> > diff --git
> > a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> > b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> > new file mode 100644
> > index 0000000..11e732a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> > @@ -0,0 +1,65 @@
> > +NXP i.MX System Controller Firmware (SCFW)
> > +--------------------------------------------------------------------
> > +
> > +The System Controller Firmware (SCFW) is a low-level system function
> > +which runs on a dedicated Cortex-M core to provide power, clock, and
> > +resource management. It exists on some i.MX8 processors. e.g. i.MX8QM
> > +(QM, QP), and i.MX8QX (QXP, DX).
> > +
> > +The AP communicates with the SC using a multi-ported MU module found
> > +in the LSIO subsystem. The current definition of this MU module
> > +provides
> > +5 remote AP connections to the SC to support up to 5 execution
> > +environments (TZ, HV, standard Linux, etc.). The SC side of this MU
> > +module interfaces with the LSIO DSC IP bus. The SC firmware will
> > +communicate with this MU using the MSI bus.
> > +
> > +System Controller Device Node:
> > +=============================
> 
> Need to specify this is a child of /firmware node.
> 

Got it.

> > +
> > +Required properties:
> > +-------------------
> > +- compatible: should be "fsl,imx-scu"
> > +- mboxes: List of phandle of MU mailbox. Should be one of LSIO
> > +	  MU0~M4 for imx8qxp and imx8qm. Users need to make
> > +	  sure not use the one which is conflict with other
> > +	  execution environments. e.g. ATF.
> > +
> > +Examples:
> > +--------
> > +lsio_mu1: mailbox@5d1c0000 {
> > +	compatible = "fsl,imx8qxp-mu";
> > +	reg = <0x0 0x5d1c0000 0x0 0x10000>;
> > +	interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>;
> > +	#mbox-cells = <0>;
> > +};
> > +
> > +scu {
> > +	compatible = "fsl,imx-scu";
> > +	mboxes = <&lsio_mu1>;
> > +};
> > +
> > +
> > +i.MX SCU Client Device Node:
> > +=========================
> > +
> > +Client nodes are maintained as children of the relevant IMX-SCU device
> node.
> > +
> > +Example (imx8qxp):
> > +-------------
> > +scu {
> > +	compatible = "fsl,imx-scu";
> > +	...
> > +
> > +	clk: clk {
> > +		...
> > +	};
> > +
> > +	iomuxc: iomuxc {
> > +		...
> > +	};
> > +
> > +	imx8qx-pm {
> > +		...
> > +	}
> > +	...
> 
> If you plan to have child nodes, you need to document them now. However,
> other than pinctrl, you probably don't need child nodes. The parent node can
> be a clock and power-domain provider.
> 

Sorry I did not get the point.
This is referred to what TI does (arm scpi binding seems similar):
Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
Documentation/devicetree/bindings/clock/ti,sci-clk.txt

arch/arm/boot/dts/keystone-k2g.dtsi
pmmc: pmmc@2921c00 {
        compatible = "ti,k2g-sci";
	....

        k2g_pds: power-controller {
                compatible = "ti,sci-pm-domain";
                #power-domain-cells = <1>;
        };

        k2g_clks: clocks {
                compatible = "ti,k2g-sci-clk";
                #clock-cells = <2>;
        };

        k2g_reset: reset-controller {
                compatible = "ti,sci-reset";
                #reset-cells = <2>;
        };
};

For i.MX SCU, it's quite similar:
firmware {
        scu {
                compatible = "fsl,imx-scu";
                mboxes = <&lsio_mu1>;

                clk: clk {
                        compatible = "fsl,imx8qxp-clk";
                        #clock-cells = <1>;
                        mboxes = <&lsio_mu2>; //optinoal
                };

                iomuxc: iomuxc {
                        compatible = "fsl,imx8qxp-iomuxc";
                        fsl,mu = <&lsio_mu3>; //optinoal
                };

                imx8qx-pm {
                        compatible = "fsl,scu-pd";
                        #address-cells = <1>;
                        #size-cells = <0>;
                        fsl,mu = <&lsio_mu4>; //optinoal

                        pd_lsio: PD_LSIO {
                                #power-domain-cells = <0>;
                                #address-cells = <1>;
                                #size-cells = <0>;

                                pd_lsio_pwm0: lsio_pwm0@SC_R_PWM_0 {
                                        reg = <SC_R_PWM_0>;
                                        #power-domain-cells = <0>;
                                        power-domains = <&pd_lsio>;
                                };
                                ...
                        };

                        pd_dma: dma_power_domain {
                                #power-domain-cells = <0>;
                                #address-cells = <1>;
                                #size-cells = <0>;
                        };
                        ...
                };

                other_funcs {
                        compatible = "xxx";
                        ...
                };
        };
};

1. The child allows to specify a different mbox to use (planned optional)
2. we have separate clk, pinctrl, power domain drivers and etc.
3. we need define child pd nodes under the power domain node

Am I understanding correct that you are saying we can merge the clk and pd
driver into scu driver then scu behaviors both clk and pd provider?
It means we need implement both clk driver and pd driver in scu driver
which may bloat the scu driver a lot and make us no able to specify different
mbox for subnodes.
And clk driver has a lot clock type definitions which is under clock drivers.
Not sure suitable to be moved into scu.
See:
https://patchwork.ozlabs.org/cover/905920/

It will be like:
firmware {                              
        scu {                           
                compatible = "fsl,imx-scu";
                mboxes = <&lsio_mu1>;
                #clock-cells = <1>;
                #address-cells = <1>;
                #size-cells = <0>;

                iomuxc: iomuxc {
                        compatible = "fsl,imx8qxp-iomuxc";
                        fsl,mu = <&lsio_mu3>;
                };

                other_funcs: {
                        compatible = "thermal...";
                        ...
                };

                pd_lsio: PD_LSIO {
                        #power-domain-cells = <0>;
                        #address-cells = <1>;
                        #size-cells = <0>;

                        pd_lsio_pwm0: lsio_pwm0@SC_R_PWM_0 {
                                reg = <SC_R_PWM_0>;
                                #power-domain-cells = <0>;
                                power-domains = <&pd_lsio>;
                        };
                        ...
                };

                pd_vpu: vpu-power-domain@SC_R_VPU_PID0 {
                        reg = <SC_R_VPU_PID0>;
                        #power-domain-cells = <0>;
                        #address-cells = <1>;
                        #size-cells = <0>;
                };

                pd_xxx {
                        ...
                }
                ....
        };
};

How would you suggest for such case?

Regards
Dong Aisheng

> Rob