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 |
