linux-kernel - Re: [PATCH] net: fec: Refactor MAC reset to function

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] [day] [month] [year] [list]

Message-ID: <20250205134824.GF554665@kernel.org>
Date: Wed, 5 Feb 2025 13:48:24 +0000
From: Simon Horman <horms@...nel.org>
To: Csókás, Bence <csokas.bence@...lan.hu>
Cc: Laurent Badel <laurentbadel@...on.com>,
	Jakub Kicinski <kuba@...nel.org>, Andrew Lunn <andrew@...n.ch>,
	imx@...ts.linux.dev, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>,
	Jacob Keller <jacob.e.keller@...el.com>,
	Wei Fang <wei.fang@....com>, Shenwei Wang <shenwei.wang@....com>,
	Clark Wang <xiaoning.wang@....com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH] net: fec: Refactor MAC reset to function

On Wed, Feb 05, 2025 at 01:28:32PM +0000, Simon Horman wrote:
> On Tue, Feb 04, 2025 at 10:36:03AM +0100, Csókás, Bence wrote:
> > The core is reset both in `fec_restart()` (called on link-up) and
> > `fec_stop()` (going to sleep, driver remove etc.). These two functions
> > had their separate implementations, which was at first only a register
> > write and a `udelay()` (and the accompanying block comment). However,
> > since then we got soft-reset (MAC disable) and Wake-on-LAN support, which
> > meant that these implementations diverged, often causing bugs.
> > 
> > For instance, as of now, `fec_stop()` does not check for
> > `FEC_QUIRK_NO_HARD_RESET`, meaning the MII/RMII mode is cleared on eg.
> > a PM power-down event; and `fec_restart()` missed the refactor renaming
> > the "magic" constant `1` to `FEC_ECR_RESET`.
> > 
> > To harmonize current implementations, and eliminate this source of
> > potential future bugs, refactor implementation to a common function.
> > 
> > Reviewed-by: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
> > Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>
> > Fixes: c730ab423bfa ("net: fec: Fix temporary RMII clock reset on link up")
> > Fixes: ff049886671c ("net: fec: Refactor: #define magic constants")
> > Signed-off-by: Csókás, Bence <csokas.bence@...lan.hu>
> 
> Reviewed-by: Simon Horman <horms@...nel.org>

Sorry, I think I have to take that back for now.

Unfortunately when I was looking over this I failed to notice: that there
is a newer version of this patch (v3); the following response from Jakub to
v3; and, the response from Laurent that he is referring to.

  "Laurent responded to v1 saying this was intentional. Please give more
   details on how problem you're seeing and on what platforms. Otherwise
   this is not a fix but refactoring.

  "Please don't post new versions in-reply-to, and add lore links to
   the previous version in the changelog.

  Link: https://lore.kernel.org/netdev/20250204074504.523c794c@kernel.org/