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 |