All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Horman <horms@kernel.org>
To: Wei Fang <wei.fang@nxp.com>
Cc: Jakub Kicinski <kuba@kernel.org>,
	Claudiu Manoil <claudiu.manoil@nxp.com>,
	Vladimir Oltean <vladimir.oltean@nxp.com>,
	Clark Wang <xiaoning.wang@nxp.com>,
	"andrew+netdev@lunn.ch" <andrew+netdev@lunn.ch>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"edumazet@google.com" <edumazet@google.com>,
	"pabeni@redhat.com" <pabeni@redhat.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"imx@lists.linux.dev" <imx@lists.linux.dev>
Subject: Re: [PATCH v2 net-next 0/3] change some statistics to 64-bit
Date: Wed, 25 Jun 2025 17:34:59 +0100	[thread overview]
Message-ID: <20250625163459.GD152961@horms.kernel.org> (raw)
In-Reply-To: <PAXPR04MB8510EDB597AD25F666C450ED887BA@PAXPR04MB8510.eurprd04.prod.outlook.com>

On Wed, Jun 25, 2025 at 02:22:54AM +0000, Wei Fang wrote:
> > On Tue, 24 Jun 2025 18:15:45 +0800 Wei Fang wrote:
> > > The port MAC counters of ENETC are 64-bit registers and the statistics
> > > of ethtool are also u64 type, so add enetc_port_rd64() helper function
> > > to read 64-bit statistics from these registers, and also change the
> > > statistics of ring to unsigned long type to be consistent with the
> > > statistics type in struct net_device_stats.
> > 
> > this series adds almost 100 sparse warnings please trying building it with C=1
> > --
> > pw-bot: cr
> 
> Hi Jakub,
> 
> Simon has posted a patch [1] to fix the sparse warnings. Do I need to wait until
> Simon's patch is applied to the net-next tree and then resend this patch set?
> 
> [1] https://lore.kernel.org/imx/20250624-etnetc-le-v1-1-a73a95d96e4e@kernel.org/

Yes, I have confirmed that with patch[1] applied this patch-set
does not introduce any Sparse warnings (in my environment).

I noticed the Sparse warnings that are otherwise introduced when reviewing
v1 of this patchset which is why I crated patch[1].

The issue is that there is are long standing Sparse warnings - which
highlight a driver bug, albeit one that doesn't manifest with in tree
users. They is due to an unnecessary call to le64_to_cpu(). The warnings
are:

  .../enetc_hw.h:513:16: warning: cast to restricted __le64
  .../enetc_hw.h:513:16: warning: restricted __le64 degrades to integer
  .../enetc_hw.h:513:16: warning: cast to restricted __le64

Patches 2/3 and 3/3 multiply the incidence of the above 3 warnings because
they increase the callers of the inline function where the problem lies.

But I'd argue that, other than noise, they don't make things worse.
The bug doesn't manifest for in-tree users (and if it did, it would
have been manifesting anyway).

So I'd advocate accepting this series (or not) independent of resolving
the Sparse warnings. Which should disappear when patch[1], or some variant
thereof, is accepted (via net or directly into net-next).

  reply	other threads:[~2025-06-25 16:35 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-24 10:15 [PATCH v2 net-next 0/3] change some statistics to 64-bit Wei Fang
2025-06-24 10:15 ` [PATCH v2 net-next 1/3] net: enetc: change the statistics of ring to unsigned long type Wei Fang
2025-06-24 16:55   ` Frank Li
2025-06-25 16:24   ` Simon Horman
2025-06-24 10:15 ` [PATCH v2 net-next 2/3] net: enetc: separate 64-bit counters from enetc_port_counters Wei Fang
2025-06-24 16:59   ` Frank Li
2025-06-25 16:24   ` Simon Horman
2025-06-24 10:15 ` [PATCH v2 net-next 3/3] net: enetc: read 64-bit statistics from port MAC counters Wei Fang
2025-06-24 17:00   ` Frank Li
2025-06-25 16:25   ` Simon Horman
2025-06-25  1:11 ` [PATCH v2 net-next 0/3] change some statistics to 64-bit Jakub Kicinski
2025-06-25  2:22   ` Wei Fang
2025-06-25 16:34     ` Simon Horman [this message]
2025-06-25 20:32       ` Jakub Kicinski
2025-06-26  1:40         ` Wei Fang
2025-06-26  7:23         ` Simon Horman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250625163459.GD152961@horms.kernel.org \
    --to=horms@kernel.org \
    --cc=andrew+netdev@lunn.ch \
    --cc=claudiu.manoil@nxp.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=imx@lists.linux.dev \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=vladimir.oltean@nxp.com \
    --cc=wei.fang@nxp.com \
    --cc=xiaoning.wang@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.