Received-SPF: pass (google.com: domain of linux-kernel+bounces-34813-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Tue, 23 Jan 2024 08:09:51 +0100
From: Sascha Hauer <s.hauer@pengutronix.de>
To: Peng Fan <peng.fan@nxp.com>
Cc: "Peng Fan (OSS)" <peng.fan@oss.nxp.com>,
	Jassi Brar <jassisinghbrar@gmail.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Aisheng Dong <aisheng.dong@nxp.com>,
	Shawn Guo <shawnguo@kernel.org>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	dl-linux-imx <linux-imx@nxp.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v2 2/3] mailbox: imx: get RR/TR registers num from
 Parameter register
Message-ID: <20240123070951.GA93665@pengutronix.de>
References: <20240122-imx-mailbox-v2-0-7b3c80333b92@nxp.com>
 <20240122-imx-mailbox-v2-2-7b3c80333b92@nxp.com>
 <20240122090330.GR4700@pengutronix.de>
 <DU0PR04MB9417852AC47FA4BCFB38560F88742@DU0PR04MB9417.eurprd04.prod.outlook.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <DU0PR04MB9417852AC47FA4BCFB38560F88742@DU0PR04MB9417.eurprd04.prod.outlook.com>
User-Agent: Mutt/1.10.1 (2018-07-13)

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 |