Received-SPF: pass (google.com: domain of linux-kernel+bounces-34598-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
From: Peng Fan <peng.fan@nxp.com>
To: Sascha Hauer <s.hauer@pengutronix.de>, "Peng Fan (OSS)"
	<peng.fan@oss.nxp.com>
CC: 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
Thread-Topic: [PATCH v2 2/3] mailbox: imx: get RR/TR registers num from
 Parameter register
Thread-Index: AQHaTPpbp7xyUI3B60Gv2ce5VFbv/7DliccAgAEWP0A=
Date: Tue, 23 Jan 2024 01:42:03 +0000
Message-ID:
 <DU0PR04MB9417852AC47FA4BCFB38560F88742@DU0PR04MB9417.eurprd04.prod.outlook.com>
References: <20240122-imx-mailbox-v2-0-7b3c80333b92@nxp.com>
 <20240122-imx-mailbox-v2-2-7b3c80333b92@nxp.com>
 <20240122090330.GR4700@pengutronix.de>
In-Reply-To: <20240122090330.GR4700@pengutronix.de>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0:
 =?us-ascii?Q?g8kQGoRuPqUdxjgF1kvY33he+q5ZcM+xgfrKQTQGaRoAe/QUyXOI/qP0Yqs+?=
 =?us-ascii?Q?fK2FMqm4puEJMTz5Q0/jlE6h27HQY8xYycSfaz88vj31VFHMt7l+bHzerpgN?=
 =?us-ascii?Q?cJiiqAABgLP50UvejbA+mnY+MKPBtTUPaDMNm+fjvQgtOVKRzMWHLljHznMU?=
 =?us-ascii?Q?5HHGtXqyOoVZljmgutQi6A1yXPKNuXfmLBrU9NQIc/hIg+UfefNktqwbENyB?=
 =?us-ascii?Q?gR9FXXlFXf7Ip8dYcTz6saD5cF+Mr1QPDJL/GCnBvvRVEpJ2iyAjq+MAlU3A?=
 =?us-ascii?Q?v4vW5HaUlEEaOtfKLiJH8W3e0ZJxKZDFap8Jg7Nu7th50qw9w+8CX6XYlWFk?=
 =?us-ascii?Q?2+92mCzBKqdV49ikQlpT0vRbnGjvC8EnbEz37LoJtn8zq6tS8oZomw+K2meU?=
 =?us-ascii?Q?SY4WMldQcVx8fxM3uwa+mQttVgImoQQk+WUlREk3QnuEBgU6dRzg+SYEcB8W?=
 =?us-ascii?Q?IW7giFilnXb7fvggidZpEtoA8UsdPKB/rl33+G023209ZTbtptoT3ZgsPxoa?=
 =?us-ascii?Q?yqQFCW4gUqQx1o8BPZfxPYbRCD7IyR2WyGvMysesnlVZCRpdPnwSgM52L2ud?=
 =?us-ascii?Q?AWax+7f0UwnsFlYgQSeIuCC2CXzw7nU5b2LqaSHxo+2ef2GXn8dfuo7X/SLr?=
 =?us-ascii?Q?KK4iHqCLfcmveFD3aYBb5ibsysAHi201bOIAMbiUmb2tnJuYsghwyuC4g0d0?=
 =?us-ascii?Q?i6aAAEIQOpQCA1ibK3FCYpMsm6fLZftMnJJZuc2jPEb9D/Z1hGV+7u5ZnXtY?=
 =?us-ascii?Q?r2CQHmTjEhIgB3Li64pzNkZc2JP5lyYDKgg83x/mUE8wqu/xZq2lMVLAGNGS?=
 =?us-ascii?Q?ZoEWJzRKaT4smYu98FkfrUiNGsxynJZMNDjvFgD9zTaaAfHbfPDpy7xsD+a3?=
 =?us-ascii?Q?XfV79e0GyN8ZOuItwmrDtx4wyQ649qrGzA9vZrTK+E6nIwZ75REjpvqfquME?=
 =?us-ascii?Q?Om8ulm2RAtsM04Pi9WzRX9iapHAXCajr4LAFucBH3Y4LIVaD+rIqkQ7t10ve?=
 =?us-ascii?Q?OzT55kobKnxrsqXIcdUZb6/Mb9wRwF59UgcyrMX9lnZ8Ux1TZgawhRiuWCrv?=
 =?us-ascii?Q?shv4bIjx3dS0eHrlkiGP0uCjQzzXbSnF3oSvHT1nP28el+/g0eTBArka6KqI?=
 =?us-ascii?Q?/UBWFc3qBQEQgjiTPr+09K51L5SUxVllhFR9vH0kgAo6XqdNXRVdUBi0IFZI?=
 =?us-ascii?Q?hJOpomUdaq1mJzuVZB7pokpCJZeWmMcujV9TNREFBGMGx8kcXQLRDTQf6UXz?=
 =?us-ascii?Q?Dg/PWtg4S2+Ys+liEgiRPyuYsm+RtJxeEmceZdW6ylw6VlR0a/agKK8Oy7xK?=
 =?us-ascii?Q?wha6yg02sVGSvQ9GjwvUFHZMXqKygIa5cTT746Hh3uQnXzkavx0mKUsPCLoZ?=
 =?us-ascii?Q?vXoDFq6Or7lzQnxkvrRFVb9W46I5sW9z8hXHDevkr2vRPNXmmbtcn7t1dvs7?=
 =?us-ascii?Q?r/HqdcHGl7P8YBwXMW54CQlAPMFT5YaQQ6qz421zkOptgrbNRSnSl943IvI7?=
 =?us-ascii?Q?wRGXqlldC+knn2vGq8VdxQvvvxCFEp2cQtFbLjrYPrL3R8KEu/9ZQwctWLPm?=
 =?us-ascii?Q?PY3DPPVilGqP5zZEBSU=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DU0PR04MB9417.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 311ef9dd-aa43-48af-bd08-08dc1bb47fc5
X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Jan 2024 01:42:03.1197
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: pX2/81MijmHVB+d2xACPdZnXvtBurKKmAxCen4vzzEBsg9eH+250ykVPCNulwZLsdeuZx0CWvjV0ePE6OaR3AA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM9PR04MB8355

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 =3D data;
> > +   u32 num_tr =3D 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 =3D ((struct imx_s4_rpc_msg_max *)data)->hdr.size;
> >             max_size =3D sizeof(struct imx_s4_rpc_msg_max);
> > -           num_tr =3D 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 =3D ((struct imx_sc_rpc_msg_max *)data)->hdr.size;
> >             max_size =3D sizeof(struct imx_sc_rpc_msg_max);
> > -           num_tr =3D 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 =3D priv->num_rr;
> >
> >     data =3D (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 =3D 1; i < size; i++) {
> >             ret =3D 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++ =3D imx_mu_read(priv, priv->dcfg->xRR + (i % 4) * 4=
);
> > +           *data++ =3D 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 =3D imx_mu_read(priv, IMX_MU_V2_PAR_OFF);
> > +           priv->num_tr =3D FIELD_GET(IMX_MU_V2_TR_MASK, val);
> > +           priv->num_rr =3D FIELD_GET(IMX_MU_V2_RR_MASK, val);
> > +   } else {
> > +           priv->num_tr =3D 4;
> > +           priv->num_rr =3D 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 th=
is
> 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 ra=
ther
> 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=3D05%7C02%7Cpeng.fan%40nxp.com%7C8e3e1d53fd
> 694029ddc708dc1b290bd4%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C0%7C638415110314502701%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi
> MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C30
> 00%7C%7C%7C&sdata=3Di9Q4SuR%2BwgOGLodJtZJlgMYngyikZNP5ktxiNqzMf
> WM%3D&reserved=3D0  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    =
|
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 =
|