Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Vladimir Oltean <vladimir.oltean@nxp.com>
To:     Colin Foster <colin.foster@in-advantage.com>
CC:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        Jakub Kicinski <kuba@kernel.org>,
        "David S. Miller" <davem@davemloft.net>,
        "UNGLinuxDriver@microchip.com" <UNGLinuxDriver@microchip.com>,
        Alexandre Belloni <alexandre.belloni@bootlin.com>,
        Claudiu Manoil <claudiu.manoil@nxp.com>
Subject: Re: [PATCH v6 net-next 2/5] net: mscc: ocelot: remove unnecessary
 stat reading from ethtool
Thread-Topic: [PATCH v6 net-next 2/5] net: mscc: ocelot: remove unnecessary
 stat reading from ethtool
Thread-Index: AQHYHjTeSRre4+TdMUarRWdg03eF1ayMlzsA
Date:   Thu, 10 Feb 2022 10:34:47 +0000
Message-ID: <20220210103446.3rdnulyk5p7tzgn6@skbuf>
References: <20220210041345.321216-1-colin.foster@in-advantage.com>
 <20220210041345.321216-3-colin.foster@in-advantage.com>
In-Reply-To: <20220210041345.321216-3-colin.foster@in-advantage.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?C8lpwtAFhG4DYRdl4OfZvdpBsfJFFTLHrF5dWIHbOpXwuyxKWg4TX/4xm0O/?=
 =?us-ascii?Q?5F6U6A3l2dp0AZvZhKY8ZuoU7qUI1OFllqYIlb6cuNlZ48nUD62X7vm4Wqe+?=
 =?us-ascii?Q?blaYFDIj6l0JthP67nL9S7q6X9P6noKRZqnSAJeNr99Dv5cMU57KC+jSQVrb?=
 =?us-ascii?Q?SGIwOhtVLYEHTDgMmy5820M/7xlThm1S3/dmsAnQBlKh5ZAJALt7hM2vSNXX?=
 =?us-ascii?Q?ciE84m2S2fCh8YCaNp19vqnhmBoWSgogdNRnlWP3OS9XP5g30Q4qTeacDvyi?=
 =?us-ascii?Q?czq8RxwFblTm7b9UrN+Pri+X78mwzfbHhBA/6i6Xnhf/rVt7kWKGkxL4J9Fd?=
 =?us-ascii?Q?+B1Gi93z18pFwRtRjvgQiGmPmj9S2SwDgUlsLtVz0Jz3FXR4o0lLHmz+myoT?=
 =?us-ascii?Q?plqxUoJ/NeBSPUpwc42d9V2z1/GbEVVM82Vh/g8kKwfPbpjA/eDglEG+eFit?=
 =?us-ascii?Q?l0LjAI4nR9AOvHQFaahk/r2qTqYn0z0z6B5AtHaZSNZpz+vgnEYbW4K6W6br?=
 =?us-ascii?Q?gSDRgtL0SJ7EZrjc/+CC/loLJG0IGKbX/fN7r9Fdarb0CEXRLKTWXg+P7XsW?=
 =?us-ascii?Q?7NZPwBxkAQ0dNqCc11mcUc0sce7u5p93Kvn98V40VxJp7vTJCuKRdrtqxLGL?=
 =?us-ascii?Q?5cbVr9U3xTHiu7DXrML7IXV6cK+yYA6TetFf1Q9uGGgxqmoddHmDcrNup0tb?=
 =?us-ascii?Q?+J0iDRcGnfULDf6sdP+TvVK1hYYBxw026Wx9dhjI1n8ThwbvVqJ+0t9yjg+P?=
 =?us-ascii?Q?r+Ge+F0koDrMnzw5uxg8q4A25XkgF82YkIz0sa2VX7T3kySSs4TisQPp7o7k?=
 =?us-ascii?Q?Q3475K0hjM0mhcWe1PUHBuuEkLGTTk3Jnd8uI38HFXTZGqnyVZ/QOPuNeNji?=
 =?us-ascii?Q?eVlkUrZwnvmPszcw8CFeaIETeASoLzVt8pH3fHoxbg8lT8ahE/XaJv6ITCH2?=
 =?us-ascii?Q?1yXChUvRD2XRbtzT0oP0WCl7IlZ9bCCrFzyeiZMqOHbOmb1qlq2NPDf3rksI?=
 =?us-ascii?Q?lCsO3Kl84B2/niNoyX5+1NGFiD3YBP1MwzjkN2Fg8UUCdGQpyAJpEQV4QGsV?=
 =?us-ascii?Q?ghiUAMwLfgcweH+O2ZqdRdNa1gb9ZG+jRoct2CvQX8D+GDxUQNGoNaxkaayA?=
 =?us-ascii?Q?YxrO531Rzr4DApzeVlNCTiyJWD86NekR1U9Yg1xOvQg+QwRTKZB5NFkwr6lB?=
 =?us-ascii?Q?SgRmwNbRhoP77kFojOBhu6K5cMnUtAdho5OH1ymE1JvcgG3Yo+H87v1eKgrH?=
 =?us-ascii?Q?sPi4XHPqyTttOdwM9uYCGT1ochiEkOFssHMYebOmXClLO3bobYK8V+ZDhMtm?=
 =?us-ascii?Q?aKZRtCxPHuUUKw72dEL90qNuCU+Z6D3aHHWQSExif6g04Btq2/enO5Gr+sfM?=
 =?us-ascii?Q?JISwYgWpGc4xIeMsy2y26knyEEAHv9DCEtcpzjjlcSoiuIMsFNr1s57exuWM?=
 =?us-ascii?Q?ZjmTyiTLHSZrvLS7tTFrT9hOse1ZXsohM/aNVz2tpDashibd4NYC205dkDtU?=
 =?us-ascii?Q?93/P97Jk+EMh3WgEoLR8EBFgNJuqDXh4YYMIRrIEW08N94TCFG9EZ2BoPaI2?=
 =?us-ascii?Q?fhuKVl0e5jAFf921bnMWQyrEgeoT8Rw93m3HIBFJq0zqUCsOWbD8h0afwJAb?=
 =?us-ascii?Q?t+SUldOSCV3wgg4doJG7dah3qGjYBv/BeV9DxN4Ty72w?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <95D952DA4770CB4EB2594E26C2AB5817@eurprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: VI1PR04MB5136.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f8d7b709-9b7d-4c06-a29f-08d9ec80f5d6
X-MS-Exchange-CrossTenant-originalarrivaltime: 10 Feb 2022 10:34:47.2839
 (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: cX84PY6PdKC3pyTOCuZqDUer2JD01fsdobxXyy6p6qZqwn54gI2ztaNTlc+ftKB0Y5DLhfp/YaAs3IzaCAdICg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB3PR0402MB3899
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, Feb 09, 2022 at 08:13:42PM -0800, Colin Foster wrote:
> The ocelot_update_stats function only needs to read from one port, yet it
> was updating the stats for all ports. Update to only read the stats that
> are necessary.
>=20
> Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
> ---

Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>

>  drivers/net/ethernet/mscc/ocelot.c | 33 +++++++++++++++---------------
>  1 file changed, 16 insertions(+), 17 deletions(-)
>=20
> diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/ms=
cc/ocelot.c
> index 6933dff1dd37..ab36732e7d3f 100644
> --- a/drivers/net/ethernet/mscc/ocelot.c
> +++ b/drivers/net/ethernet/mscc/ocelot.c
> @@ -1738,27 +1738,24 @@ void ocelot_get_strings(struct ocelot *ocelot, in=
t port, u32 sset, u8 *data)
>  EXPORT_SYMBOL(ocelot_get_strings);
> =20
>  /* Caller must hold &ocelot->stats_lock */
> -static void ocelot_update_stats(struct ocelot *ocelot)
> +static void ocelot_update_stats_for_port(struct ocelot *ocelot, int port=
)

If you need to resend, I think a name more consistent with the rest of
the driver would be "ocelot_port_update_stats".

>  {
> -	int i, j;
> +	int j;
> =20
> -	for (i =3D 0; i < ocelot->num_phys_ports; i++) {
> -		/* Configure the port to read the stats from */
> -		ocelot_write(ocelot, SYS_STAT_CFG_STAT_VIEW(i), SYS_STAT_CFG);
> +	/* Configure the port to read the stats from */
> +	ocelot_write(ocelot, SYS_STAT_CFG_STAT_VIEW(port), SYS_STAT_CFG);
> =20
> -		for (j =3D 0; j < ocelot->num_stats; j++) {
> -			u32 val;
> -			unsigned int idx =3D i * ocelot->num_stats + j;
> +	for (j =3D 0; j < ocelot->num_stats; j++) {
> +		u32 val;
> +		unsigned int idx =3D port * ocelot->num_stats + j;
> =20
> -			val =3D ocelot_read_rix(ocelot, SYS_COUNT_RX_OCTETS,
> -					      ocelot->stats_layout[j].offset);
> +		val =3D ocelot_read_rix(ocelot, SYS_COUNT_RX_OCTETS,
> +				      ocelot->stats_layout[j].offset);
> =20
> -			if (val < (ocelot->stats[idx] & U32_MAX))
> -				ocelot->stats[idx] +=3D (u64)1 << 32;
> +		if (val < (ocelot->stats[idx] & U32_MAX))
> +			ocelot->stats[idx] +=3D (u64)1 << 32;
> =20
> -			ocelot->stats[idx] =3D (ocelot->stats[idx] &
> -					      ~(u64)U32_MAX) + val;
> -		}
> +		ocelot->stats[idx] =3D (ocelot->stats[idx] & ~(u64)U32_MAX) + val;
>  	}
>  }
> =20
> @@ -1767,9 +1764,11 @@ static void ocelot_check_stats_work(struct work_st=
ruct *work)
>  	struct delayed_work *del_work =3D to_delayed_work(work);
>  	struct ocelot *ocelot =3D container_of(del_work, struct ocelot,
>  					     stats_work);
> +	int i;
> =20
>  	mutex_lock(&ocelot->stats_lock);
> -	ocelot_update_stats(ocelot);
> +	for (i =3D 0; i < ocelot->num_phys_ports; i++)
> +		ocelot_update_stats_for_port(ocelot, i);
>  	mutex_unlock(&ocelot->stats_lock);
> =20
>  	queue_delayed_work(ocelot->stats_queue, &ocelot->stats_work,
> @@ -1783,7 +1782,7 @@ void ocelot_get_ethtool_stats(struct ocelot *ocelot=
, int port, u64 *data)
>  	mutex_lock(&ocelot->stats_lock);
> =20
>  	/* check and update now */
> -	ocelot_update_stats(ocelot);
> +	ocelot_update_stats_for_port(ocelot, port);
> =20
>  	/* Copy all counters */
>  	for (i =3D 0; i < ocelot->num_stats; i++)
> --=20
> 2.25.1
>=