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@xxxxxxx>
> 
> 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@xxxxxxx>
> ---
>  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 |