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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
 <DU0PR04MB9417852AC47FA4BCFB38560F88742@DU0PR04MB9417.eurprd04.prod.outlook.com>
Date: Tue, 23 Jan 2024 01:42:03 +0000
From: Peng Fan <peng.fan@....com>
To: Sascha Hauer <s.hauer@...gutronix.de>, "Peng Fan (OSS)"
	<peng.fan@....nxp.com>
CC: Jassi Brar <jassisinghbrar@...il.com>, Rob Herring <robh+dt@...nel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley
	<conor+dt@...nel.org>, Aisheng Dong <aisheng.dong@....com>, Shawn Guo
	<shawnguo@...nel.org>, Pengutronix Kernel Team <kernel@...gutronix.de>, Fabio
 Estevam <festevam@...il.com>, dl-linux-imx <linux-imx@....com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>
Subject: RE: [PATCH v2 2/3] mailbox: imx: get RR/TR registers num from
 Parameter register

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 |