[PATCH v2 0/3] mailbox: imx: support i.MX95 ELE/V2X MU

[PATCH v2 0/3] mailbox: imx: support i.MX95 ELE/V2X MU

[Peng Fan (OSS)]

- Add dt-bindings
- i.MX95 ELE/V2X use same register layout as i.MX8ULP S4 MU, but
  the TR/RR num is different. To make code reusable and not add too much
  macros, add runtime detect number of TR and RR by reading PAR_OFF
  registers.
- Add i.MX95 ELE/V2X MU entry in driver

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
Changes in v2:
- Support sram property and add example
- Populate the sram node in driver
- Link to v1: https://lore.kernel.org/r/20240122-imx-mailbox-v1-0-81413f655210@nxp.com

---
Peng Fan (3):
      dt-bindings: mailbox: fsl,mu: add i.MX95 Generic/ELE/V2X MU compatible
      mailbox: imx: get RR/TR registers num from Parameter register
      mailbox: imx: support i.MX95 ELE/V2X MU

 .../devicetree/bindings/mailbox/fsl,mu.yaml        | 50 ++++++++++++++++++++-
 drivers/mailbox/imx-mailbox.c                      | 51 +++++++++++++++++-----
 2 files changed, 88 insertions(+), 13 deletions(-)
---
base-commit: ad5c60d66016e544c51ed98635a74073f761f45d
change-id: 20240122-imx-mailbox-243021d12030

Best regards,
-- 
Peng Fan <peng.fan@nxp.com>

[PATCH v2 1/3] dt-bindings: mailbox: fsl,mu: add i.MX95 Generic/ELE/V2X MU compatible

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Add i.MX95 Generic, Secure Enclave and V2X Message Unit compatible string.
And some MUs has internal RAMs for SCMI shared buffer usage.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 .../devicetree/bindings/mailbox/fsl,mu.yaml        | 50 +++++++++++++++++++++-
 1 file changed, 48 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mailbox/fsl,mu.yaml b/Documentation/devicetree/bindings/mailbox/fsl,mu.yaml
index 12e7a7d536a3..d10c6fed291b 100644
--- a/Documentation/devicetree/bindings/mailbox/fsl,mu.yaml
+++ b/Documentation/devicetree/bindings/mailbox/fsl,mu.yaml
@@ -29,10 +29,14 @@ properties:
       - const: fsl,imx8ulp-mu
       - const: fsl,imx8-mu-scu
       - const: fsl,imx8-mu-seco
-      - const: fsl,imx93-mu-s4
       - const: fsl,imx8ulp-mu-s4
+      - const: fsl,imx93-mu-s4
+      - const: fsl,imx95-mu-ele
+      - const: fsl,imx95-mu-v2x
       - items:
-          - const: fsl,imx93-mu
+          - enum:
+              - fsl,imx93-mu
+              - fsl,imx95-mu
           - const: fsl,imx8ulp-mu
       - items:
           - enum:
@@ -95,6 +99,17 @@ properties:
   power-domains:
     maxItems: 1
 
+  ranges: true
+
+  "#address-cells": true
+
+  "#size-cells": true
+
+patternProperties:
+  "^sram@[a-z0-9]+":
+    $ref: /schemas/sram/sram.yaml#
+    unevaluatedProperties: false
+
 required:
   - compatible
   - reg
@@ -134,3 +149,34 @@ examples:
         interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>;
         #mbox-cells = <2>;
     };
+
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    mu2: mailbox@445b0000 {
+        #address-cells = <1>;
+        #size-cells = <1>;
+        compatible = "fsl,imx95-mu", "fsl,imx8ulp-mu";
+        reg = <0x445b0000 0x10000>;
+        interrupts = <GIC_SPI 226 IRQ_TYPE_LEVEL_HIGH>;
+        ranges;
+        #mbox-cells = <2>;
+
+        sram@445b1000 {
+            compatible = "mmio-sram";
+            reg = <0x445b1000 0x400>;
+            #address-cells = <1>;
+            #size-cells = <1>;
+            ranges = <0x0 0x445b1000 0x400>;
+
+            scmi_buf0: scmi-sram-section@0 {
+                compatible = "arm,scmi-shmem";
+                reg = <0x0 0x80>;
+            };
+
+            scmi_buf1: scmi-sram-section@80 {
+                compatible = "arm,scmi-shmem";
+                reg = <0x80 0x80>;
+            };
+        };
+    };

-- 
2.37.1

[PATCH v2 2/3] mailbox: imx: get RR/TR registers num from Parameter register

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

i.MX8ULP, i.MX93 MU has a Parameter register encoded as below:
BIT: 15 --- 8 | 7 --- 0
      RR_NUM    TR_NUM

So to make driver easy to support more variants, get the RR/TR
registers number from Parameter register.

The patch only adds support the specific MU, such as ELE MU.
For generic MU, not add support for number larger than 4.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/mailbox/imx-mailbox.c | 46 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 35 insertions(+), 11 deletions(-)

diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
index 656171362fe9..f2a21baded29 100644
--- a/drivers/mailbox/imx-mailbox.c
+++ b/drivers/mailbox/imx-mailbox.c
@@ -29,7 +29,9 @@
 #define IMX_MU_S4_CHANS		2
 #define IMX_MU_CHAN_NAME_SIZE	20
 
-#define IMX_MU_NUM_RR		4
+#define IMX_MU_V2_PAR_OFF	0x4
+#define IMX_MU_V2_TR_MASK	GENMASK(7, 0)
+#define IMX_MU_V2_RR_MASK	GENMASK(15, 8)
 
 #define IMX_MU_SECO_TX_TOUT (msecs_to_jiffies(3000))
 #define IMX_MU_SECO_RX_TOUT (msecs_to_jiffies(3000))
@@ -93,10 +95,11 @@ struct imx_mu_priv {
 	struct clk		*clk;
 	int			irq[IMX_MU_CHANS];
 	bool			suspend;
-
-	u32 xcr[IMX_MU_xCR_MAX];
-
 	bool			side_b;
+
+	u32			xcr[IMX_MU_xCR_MAX];
+	u32			num_tr;
+	u32			num_rr;
 };
 
 enum imx_mu_type {
@@ -264,18 +267,17 @@ static int imx_mu_generic_rxdb(struct imx_mu_priv *priv,
 static int imx_mu_specific_tx(struct imx_mu_priv *priv, struct imx_mu_con_priv *cp, void *data)
 {
 	u32 *arg = data;
+	u32 num_tr = priv->num_tr;
 	int i, ret;
 	u32 xsr;
-	u32 size, max_size, num_tr;
+	u32 size, max_size;
 
 	if (priv->dcfg->type & IMX_MU_V2_S4) {
 		size = ((struct imx_s4_rpc_msg_max *)data)->hdr.size;
 		max_size = sizeof(struct imx_s4_rpc_msg_max);
-		num_tr = 8;
 	} else {
 		size = ((struct imx_sc_rpc_msg_max *)data)->hdr.size;
 		max_size = sizeof(struct imx_sc_rpc_msg_max);
-		num_tr = 4;
 	}
 
 	switch (cp->type) {
@@ -324,6 +326,7 @@ static int imx_mu_specific_rx(struct imx_mu_priv *priv, struct imx_mu_con_priv *
 	int i, ret;
 	u32 xsr;
 	u32 size, max_size;
+	u32 num_rr = priv->num_rr;
 
 	data = (u32 *)priv->msg;
 
@@ -345,13 +348,13 @@ static int imx_mu_specific_rx(struct imx_mu_priv *priv, struct imx_mu_con_priv *
 
 	for (i = 1; i < size; i++) {
 		ret = readl_poll_timeout(priv->base + priv->dcfg->xSR[IMX_MU_RSR], xsr,
-					 xsr & IMX_MU_xSR_RFn(priv->dcfg->type, i % 4), 0,
+					 xsr & IMX_MU_xSR_RFn(priv->dcfg->type, i % num_rr), 0,
 					 5 * USEC_PER_SEC);
 		if (ret) {
 			dev_err(priv->dev, "timeout read idx %d\n", i);
 			return ret;
 		}
-		*data++ = imx_mu_read(priv, priv->dcfg->xRR + (i % 4) * 4);
+		*data++ = imx_mu_read(priv, priv->dcfg->xRR + (i % num_rr) * 4);
 	}
 
 	imx_mu_xcr_rmw(priv, IMX_MU_RCR, IMX_MU_xCR_RIEn(priv->dcfg->type, 0), 0);
@@ -737,11 +740,30 @@ static struct mbox_chan *imx_mu_seco_xlate(struct mbox_controller *mbox,
 	return imx_mu_xlate(mbox, sp);
 }
 
+static void imx_mu_get_tr_rr(struct imx_mu_priv *priv)
+{
+	u32 val;
+
+	if (priv->dcfg->type & IMX_MU_V2) {
+		val = imx_mu_read(priv, IMX_MU_V2_PAR_OFF);
+		priv->num_tr = FIELD_GET(IMX_MU_V2_TR_MASK, val);
+		priv->num_rr = FIELD_GET(IMX_MU_V2_RR_MASK, val);
+	} else {
+		priv->num_tr = 4;
+		priv->num_rr = 4;
+	}
+}
+
 static void imx_mu_init_generic(struct imx_mu_priv *priv)
 {
 	unsigned int i;
 	unsigned int val;
 
+	if (priv->num_rr > 4 || priv->num_tr > 4) {
+		WARN_ONCE(true, "%s not support TR/RR larger than 4\n", __func__);
+		return;
+	}
+
 	for (i = 0; i < IMX_MU_CHANS; i++) {
 		struct imx_mu_con_priv *cp = &priv->con_priv[i];
 
@@ -768,8 +790,8 @@ static void imx_mu_init_generic(struct imx_mu_priv *priv)
 	imx_mu_write(priv, val, priv->dcfg->xSR[IMX_MU_GSR]);
 
 	/* Clear any pending RSR */
-	for (i = 0; i < IMX_MU_NUM_RR; i++)
-		imx_mu_read(priv, priv->dcfg->xRR + (i % 4) * 4);
+	for (i = 0; i < priv->num_rr; i++)
+		imx_mu_read(priv, priv->dcfg->xRR + i * 4);
 }
 
 static void imx_mu_init_specific(struct imx_mu_priv *priv)
@@ -864,6 +886,8 @@ static int imx_mu_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	imx_mu_get_tr_rr(priv);
+
 	priv->side_b = of_property_read_bool(np, "fsl,mu-side-b");
 
 	priv->dcfg->init(priv);

-- 
2.37.1

[PATCH v2 3/3] mailbox: imx: support i.MX95 ELE/V2X MU

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Add i.MX95 ELE/V2X MU support, its register layout is same as
i.MX8ULP, but the Parameter registers would show different
TR/RR. Since the driver already supports get TR/RR from Parameter
registers, not hardcoding the number, this patch just add
the compatible entry to reuse i.MX8ULP S4 cfg data.

To use the internal SRAM, need populate its sub-nodes.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/mailbox/imx-mailbox.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
index f2a21baded29..8506f92c0238 100644
--- a/drivers/mailbox/imx-mailbox.c
+++ b/drivers/mailbox/imx-mailbox.c
@@ -15,6 +15,7 @@
 #include <linux/mailbox_controller.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/suspend.h>
@@ -907,6 +908,8 @@ static int imx_mu_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	of_platform_populate(dev->of_node, NULL, NULL, dev);
+
 	pm_runtime_enable(dev);
 
 	ret = pm_runtime_resume_and_get(dev);
@@ -1018,6 +1021,8 @@ static const struct of_device_id imx_mu_dt_ids[] = {
 	{ .compatible = "fsl,imx8ulp-mu", .data = &imx_mu_cfg_imx8ulp },
 	{ .compatible = "fsl,imx8ulp-mu-s4", .data = &imx_mu_cfg_imx8ulp_s4 },
 	{ .compatible = "fsl,imx93-mu-s4", .data = &imx_mu_cfg_imx93_s4 },
+	{ .compatible = "fsl,imx95-mu-ele", .data = &imx_mu_cfg_imx8ulp_s4 },
+	{ .compatible = "fsl,imx95-mu-v2x", .data = &imx_mu_cfg_imx8ulp_s4 },
 	{ .compatible = "fsl,imx8-mu-scu", .data = &imx_mu_cfg_imx8_scu },
 	{ .compatible = "fsl,imx8-mu-seco", .data = &imx_mu_cfg_imx8_seco },
 	{ },

-- 
2.37.1

Re: [PATCH v2 2/3] mailbox: imx: get RR/TR registers num from Parameter register

[Sascha Hauer]

Hi Peng,

On Mon, Jan 22, 2024 at 02:19:24PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> i.MX8ULP, i.MX93 MU has a Parameter register encoded as below:
> BIT: 15 --- 8 | 7 --- 0
>       RR_NUM    TR_NUM
> 
> So to make driver easy to support more variants, get the RR/TR
> registers number from Parameter register.
> 
> The patch only adds support the specific MU, such as ELE MU.
> For generic MU, not add support for number larger than 4.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/mailbox/imx-mailbox.c | 46 ++++++++++++++++++++++++++++++++-----------
>  1 file changed, 35 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
> index 656171362fe9..f2a21baded29 100644
> --- a/drivers/mailbox/imx-mailbox.c
> +++ b/drivers/mailbox/imx-mailbox.c
> @@ -29,7 +29,9 @@
>  #define IMX_MU_S4_CHANS		2
>  #define IMX_MU_CHAN_NAME_SIZE	20
>  
> -#define IMX_MU_NUM_RR		4
> +#define IMX_MU_V2_PAR_OFF	0x4
> +#define IMX_MU_V2_TR_MASK	GENMASK(7, 0)
> +#define IMX_MU_V2_RR_MASK	GENMASK(15, 8)
>  
>  #define IMX_MU_SECO_TX_TOUT (msecs_to_jiffies(3000))
>  #define IMX_MU_SECO_RX_TOUT (msecs_to_jiffies(3000))
> @@ -93,10 +95,11 @@ struct imx_mu_priv {
>  	struct clk		*clk;
>  	int			irq[IMX_MU_CHANS];
>  	bool			suspend;
> -
> -	u32 xcr[IMX_MU_xCR_MAX];
> -
>  	bool			side_b;
> +
> +	u32			xcr[IMX_MU_xCR_MAX];
> +	u32			num_tr;
> +	u32			num_rr;
>  };
>  
>  enum imx_mu_type {
> @@ -264,18 +267,17 @@ static int imx_mu_generic_rxdb(struct imx_mu_priv *priv,
>  static int imx_mu_specific_tx(struct imx_mu_priv *priv, struct imx_mu_con_priv *cp, void *data)
>  {
>  	u32 *arg = data;
> +	u32 num_tr = priv->num_tr;
>  	int i, ret;
>  	u32 xsr;
> -	u32 size, max_size, num_tr;
> +	u32 size, max_size;
>  
>  	if (priv->dcfg->type & IMX_MU_V2_S4) {
>  		size = ((struct imx_s4_rpc_msg_max *)data)->hdr.size;
>  		max_size = sizeof(struct imx_s4_rpc_msg_max);
> -		num_tr = 8;

This change looks unexpected here. num_tr used to be 8 here and now
becomes 4 at maximum. Was this a bug? If yes, this deserves a separate
patch with an explanation what was wrong here.

>  	} else {
>  		size = ((struct imx_sc_rpc_msg_max *)data)->hdr.size;
>  		max_size = sizeof(struct imx_sc_rpc_msg_max);
> -		num_tr = 4;
>  	}
>  
>  	switch (cp->type) {
> @@ -324,6 +326,7 @@ static int imx_mu_specific_rx(struct imx_mu_priv *priv, struct imx_mu_con_priv *
>  	int i, ret;
>  	u32 xsr;
>  	u32 size, max_size;
> +	u32 num_rr = priv->num_rr;
>  
>  	data = (u32 *)priv->msg;
>  
> @@ -345,13 +348,13 @@ static int imx_mu_specific_rx(struct imx_mu_priv *priv, struct imx_mu_con_priv *
>  
>  	for (i = 1; i < size; i++) {
>  		ret = readl_poll_timeout(priv->base + priv->dcfg->xSR[IMX_MU_RSR], xsr,
> -					 xsr & IMX_MU_xSR_RFn(priv->dcfg->type, i % 4), 0,
> +					 xsr & IMX_MU_xSR_RFn(priv->dcfg->type, i % num_rr), 0,
>  					 5 * USEC_PER_SEC);
>  		if (ret) {
>  			dev_err(priv->dev, "timeout read idx %d\n", i);
>  			return ret;
>  		}
> -		*data++ = imx_mu_read(priv, priv->dcfg->xRR + (i % 4) * 4);
> +		*data++ = imx_mu_read(priv, priv->dcfg->xRR + (i % num_rr) * 4);
>  	}
>  
>  	imx_mu_xcr_rmw(priv, IMX_MU_RCR, IMX_MU_xCR_RIEn(priv->dcfg->type, 0), 0);
> @@ -737,11 +740,30 @@ static struct mbox_chan *imx_mu_seco_xlate(struct mbox_controller *mbox,
>  	return imx_mu_xlate(mbox, sp);
>  }
>  
> +static void imx_mu_get_tr_rr(struct imx_mu_priv *priv)
> +{
> +	u32 val;
> +
> +	if (priv->dcfg->type & IMX_MU_V2) {
> +		val = imx_mu_read(priv, IMX_MU_V2_PAR_OFF);
> +		priv->num_tr = FIELD_GET(IMX_MU_V2_TR_MASK, val);
> +		priv->num_rr = FIELD_GET(IMX_MU_V2_RR_MASK, val);
> +	} else {
> +		priv->num_tr = 4;
> +		priv->num_rr = 4;
> +	}
> +}
> +
>  static void imx_mu_init_generic(struct imx_mu_priv *priv)
>  {
>  	unsigned int i;
>  	unsigned int val;
>  
> +	if (priv->num_rr > 4 || priv->num_tr > 4) {
> +		WARN_ONCE(true, "%s not support TR/RR larger than 4\n", __func__);
> +		return;
> +	}

imx_mu_init_generic() is not called for all device types, nevertheless
this should be treated as an error for all device types, so this check
should be done where the variables are initialized. Also, please return
an error rather than just issue a warning.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

RE: [PATCH v2 2/3] mailbox: imx: get RR/TR registers num from Parameter register

[Peng Fan]

Hi Sascha,

> Subject: Re: [PATCH v2 2/3] mailbox: imx: get RR/TR registers num from
> Parameter register
>
> Hi Peng,

[snip]
>
> >  };
> >
> >  enum imx_mu_type {
> > @@ -264,18 +267,17 @@ static int imx_mu_generic_rxdb(struct
> > imx_mu_priv *priv,  static int imx_mu_specific_tx(struct imx_mu_priv
> > *priv, struct imx_mu_con_priv *cp, void *data)  {
> >     u32 *arg = data;
> > +   u32 num_tr = priv->num_tr;
> >     int i, ret;
> >     u32 xsr;
> > -   u32 size, max_size, num_tr;
> > +   u32 size, max_size;
> >
> >     if (priv->dcfg->type & IMX_MU_V2_S4) {
> >             size = ((struct imx_s4_rpc_msg_max *)data)->hdr.size;
> >             max_size = sizeof(struct imx_s4_rpc_msg_max);
> > -           num_tr = 8;
>
> This change looks unexpected here. num_tr used to be 8 here and now
> becomes 4 at maximum. Was this a bug? If yes, this deserves a separate
> patch with an explanation what was wrong here.

Sorry, I could not follow you here.
The num_tr is switch to use priv->num_tr now. It is not changed to 4 at
maximum, it is just use priv->num_tr to avoid hardcoding it to 8.
As of now, all platforms has IMX_MU_V2_S4 are using 8, and
the hardware register num is 8, except i.MX95 V2X MU using 4.

>
> >     } else {
> >             size = ((struct imx_sc_rpc_msg_max *)data)->hdr.size;
> >             max_size = sizeof(struct imx_sc_rpc_msg_max);
> > -           num_tr = 4;
> >     }
> >
> >     switch (cp->type) {
> > @@ -324,6 +326,7 @@ static int imx_mu_specific_rx(struct imx_mu_priv
> *priv, struct imx_mu_con_priv *
> >     int i, ret;
> >     u32 xsr;
> >     u32 size, max_size;
> > +   u32 num_rr = priv->num_rr;
> >
> >     data = (u32 *)priv->msg;
> >
> > @@ -345,13 +348,13 @@ static int imx_mu_specific_rx(struct
> imx_mu_priv
> > *priv, struct imx_mu_con_priv *
> >
> >     for (i = 1; i < size; i++) {
> >             ret = readl_poll_timeout(priv->base + priv->dcfg-
> >xSR[IMX_MU_RSR], xsr,
> > -                                    xsr & IMX_MU_xSR_RFn(priv-
> >dcfg->type, i % 4), 0,
> > +                                    xsr & IMX_MU_xSR_RFn(priv-
> >dcfg->type, i % num_rr), 0,
> >                                      5 * USEC_PER_SEC);
> >             if (ret) {
> >                     dev_err(priv->dev, "timeout read idx %d\n", i);
> >                     return ret;
> >             }
> > -           *data++ = imx_mu_read(priv, priv->dcfg->xRR + (i % 4) * 4);
> > +           *data++ = imx_mu_read(priv, priv->dcfg->xRR + (i % num_rr)
> * 4);
> >     }
> >
> >     imx_mu_xcr_rmw(priv, IMX_MU_RCR, IMX_MU_xCR_RIEn(priv->dcfg-
> >type,
> > 0), 0); @@ -737,11 +740,30 @@ static struct mbox_chan
> *imx_mu_seco_xlate(struct mbox_controller *mbox,
> >     return imx_mu_xlate(mbox, sp);
> >  }
> >
> > +static void imx_mu_get_tr_rr(struct imx_mu_priv *priv) {
> > +   u32 val;
> > +
> > +   if (priv->dcfg->type & IMX_MU_V2) {
> > +           val = imx_mu_read(priv, IMX_MU_V2_PAR_OFF);
> > +           priv->num_tr = FIELD_GET(IMX_MU_V2_TR_MASK, val);
> > +           priv->num_rr = FIELD_GET(IMX_MU_V2_RR_MASK, val);
> > +   } else {
> > +           priv->num_tr = 4;
> > +           priv->num_rr = 4;
> > +   }
> > +}
> > +
> >  static void imx_mu_init_generic(struct imx_mu_priv *priv)  {
> >     unsigned int i;
> >     unsigned int val;
> >
> > +   if (priv->num_rr > 4 || priv->num_tr > 4) {
> > +           WARN_ONCE(true, "%s not support TR/RR larger than 4\n",
> __func__);
> > +           return;
> > +   }
>
> imx_mu_init_generic() is not called for all device types, nevertheless this
> should be treated as an error for all device types, so this check should be
> done where the variables are initialized. Also, please return an error rather
> than just issue a warning.

ok, I will change the function to int return type.

Thanks,
Peng.

>
> Sascha
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       |
> http://www.p/
> engutronix.de%2F&data=05%7C02%7Cpeng.fan%40nxp.com%7C8e3e1d53fd
> 694029ddc708dc1b290bd4%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C0%7C638415110314502701%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi
> MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C30
> 00%7C%7C%7C&sdata=i9Q4SuR%2BwgOGLodJtZJlgMYngyikZNP5ktxiNqzMf
> WM%3D&reserved=0  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v2 2/3] mailbox: imx: get RR/TR registers num from Parameter register

[Sascha Hauer]

On Tue, Jan 23, 2024 at 01:42:03AM +0000, Peng Fan wrote:
> Hi Sascha,
> 
> > Subject: Re: [PATCH v2 2/3] mailbox: imx: get RR/TR registers num from
> > Parameter register
> >
> > Hi Peng,
> 
> [snip]
> >
> > >  };
> > >
> > >  enum imx_mu_type {
> > > @@ -264,18 +267,17 @@ static int imx_mu_generic_rxdb(struct
> > > imx_mu_priv *priv,  static int imx_mu_specific_tx(struct imx_mu_priv
> > > *priv, struct imx_mu_con_priv *cp, void *data)  {
> > >     u32 *arg = data;
> > > +   u32 num_tr = priv->num_tr;
> > >     int i, ret;
> > >     u32 xsr;
> > > -   u32 size, max_size, num_tr;
> > > +   u32 size, max_size;
> > >
> > >     if (priv->dcfg->type & IMX_MU_V2_S4) {
> > >             size = ((struct imx_s4_rpc_msg_max *)data)->hdr.size;
> > >             max_size = sizeof(struct imx_s4_rpc_msg_max);
> > > -           num_tr = 8;
> >
> > This change looks unexpected here. num_tr used to be 8 here and now
> > becomes 4 at maximum. Was this a bug? If yes, this deserves a separate
> > patch with an explanation what was wrong here.
> 
> Sorry, I could not follow you here.
> The num_tr is switch to use priv->num_tr now. It is not changed to 4 at
> maximum, it is just use priv->num_tr to avoid hardcoding it to 8.
> As of now, all platforms has IMX_MU_V2_S4 are using 8, and
> the hardware register num is 8, except i.MX95 V2X MU using 4.

I was confused by the warning you introduced:

> > > +   if (priv->num_rr > 4 || priv->num_tr > 4) {
> > > +           WARN_ONCE(true, "%s not support TR/RR larger than 4\n",
> > __func__);
> > > +           return;
> > > +   }

It will trigger when priv->num_tr is read as 8, so I assumed it is 4 at
maximum. Indeed just the check is wrong and you might haven't notice the
warning during testing.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

RE: [PATCH v2 2/3] mailbox: imx: get RR/TR registers num from Parameter register

[Peng Fan]

> Subject: Re: [PATCH v2 2/3] mailbox: imx: get RR/TR registers num from
> Parameter register
>
> On Tue, Jan 23, 2024 at 01:42:03AM +0000, Peng Fan wrote:
> > Hi Sascha,
> >
> > > Subject: Re: [PATCH v2 2/3] mailbox: imx: get RR/TR registers num
> > > from Parameter register
> > >
> > > Hi Peng,
> >
> > [snip]
> > >
> > > >  };
> > > >
> > > >  enum imx_mu_type {
> > > > @@ -264,18 +267,17 @@ static int imx_mu_generic_rxdb(struct
> > > > imx_mu_priv *priv,  static int imx_mu_specific_tx(struct
> > > > imx_mu_priv *priv, struct imx_mu_con_priv *cp, void *data)  {
> > > >     u32 *arg = data;
> > > > +   u32 num_tr = priv->num_tr;
> > > >     int i, ret;
> > > >     u32 xsr;
> > > > -   u32 size, max_size, num_tr;
> > > > +   u32 size, max_size;
> > > >
> > > >     if (priv->dcfg->type & IMX_MU_V2_S4) {
> > > >             size = ((struct imx_s4_rpc_msg_max *)data)->hdr.size;
> > > >             max_size = sizeof(struct imx_s4_rpc_msg_max);
> > > > -           num_tr = 8;
> > >
> > > This change looks unexpected here. num_tr used to be 8 here and now
> > > becomes 4 at maximum. Was this a bug? If yes, this deserves a
> > > separate patch with an explanation what was wrong here.
> >
> > Sorry, I could not follow you here.
> > The num_tr is switch to use priv->num_tr now. It is not changed to 4
> > at maximum, it is just use priv->num_tr to avoid hardcoding it to 8.
> > As of now, all platforms has IMX_MU_V2_S4 are using 8, and the
> > hardware register num is 8, except i.MX95 V2X MU using 4.
>
> I was confused by the warning you introduced:
>
> > > > +   if (priv->num_rr > 4 || priv->num_tr > 4) {
> > > > +           WARN_ONCE(true, "%s not support TR/RR larger than
> > > > + 4\n",
> > > __func__);
> > > > +           return;
> > > > +   }
>
> It will trigger when priv->num_tr is read as 8, so I assumed it is 4 at maximum.
> Indeed just the check is wrong and you might haven't notice the warning
> during testing.

For now, we not have platform use generic MU with TR/RR larger than 4, so
just give a warning. I could follow your suggestion to return error in V2.

The reason for not supporting 8 here is it would introduce too much change,
because of more channel numbers. So I leave it for future when there
is real need.

Thanks,
Peng.

>
> Sascha
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       |
> http://www.p/
> engutronix.de%2F&data=05%7C02%7Cpeng.fan%40nxp.com%7Cc0a6c1b5cd
> 1d4c37940708dc1be25585%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C0%7C638415906104836756%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi
> MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C30
> 00%7C%7C%7C&sdata=yIw7NjXMamXh2IQNfLFU4EBblgYlRz1rYyh4nsSpHww
> %3D&reserved=0  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v2 1/3] dt-bindings: mailbox: fsl,mu: add i.MX95 Generic/ELE/V2X MU compatible

[Krzysztof Kozlowski]

On 22/01/2024 07:19, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Add i.MX95 Generic, Secure Enclave and V2X Message Unit compatible string.
> And some MUs has internal RAMs for SCMI shared buffer usage.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  .../devicetree/bindings/mailbox/fsl,mu.yaml        | 50 +++++++++++++++++++++-
>  1 file changed, 48 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/fsl,mu.yaml b/Documentation/devicetree/bindings/mailbox/fsl,mu.yaml
> index 12e7a7d536a3..d10c6fed291b 100644
> --- a/Documentation/devicetree/bindings/mailbox/fsl,mu.yaml
> +++ b/Documentation/devicetree/bindings/mailbox/fsl,mu.yaml
> @@ -29,10 +29,14 @@ properties:
>        - const: fsl,imx8ulp-mu
>        - const: fsl,imx8-mu-scu
>        - const: fsl,imx8-mu-seco
> -      - const: fsl,imx93-mu-s4
>        - const: fsl,imx8ulp-mu-s4
> +      - const: fsl,imx93-mu-s4
> +      - const: fsl,imx95-mu-ele
> +      - const: fsl,imx95-mu-v2x
>        - items:
> -          - const: fsl,imx93-mu
> +          - enum:
> +              - fsl,imx93-mu
> +              - fsl,imx95-mu
>            - const: fsl,imx8ulp-mu
>        - items:
>            - enum:
> @@ -95,6 +99,17 @@ properties:
>    power-domains:
>      maxItems: 1
>  
> +  ranges: true
> +
> +  "#address-cells": true
> +
> +  "#size-cells": true

Please narrow the addressing.

> +
> +patternProperties:
> +  "^sram@[a-z0-9]+":

Use proper regex for unit address.  a-f

> +    $ref: /schemas/sram/sram.yaml#
> +    unevaluatedProperties: false
> +
>  required:
>    - compatible
>    - reg
> @@ -134,3 +149,34 @@ examples:
>          interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>;
>          #mbox-cells = <2>;
>      };
> +
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    mu2: mailbox@445b0000 {
> +        #address-cells = <1>;

Please follow order of properties as written in DTS coding style.

> +        #size-cells = <1>;
> +        compatible = "fsl,imx95-mu", "fsl,imx8ulp-mu";
> +        reg = <0x445b0000 0x10000>;
> +        interrupts = <GIC_SPI 226 IRQ_TYPE_LEVEL_HIGH>;
> +        ranges;
> +        #mbox-cells = <2>;
> +
> +        sram@445b1000 {
> +            compatible = "mmio-sram";
> +            reg = <0x445b1000 0x400>;
> +            #address-cells = <1>;
> +            #size-cells = <1>;
> +            ranges = <0x0 0x445b1000 0x400>;

Same here.


Best regards,
Krzysztof