netdev - Re: [PATCH net-next 0/7] net: pcs: xpcs: yet more cleanups

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

Message-ID: <tnlp4m7antrcpbscpvdzpntyjudgs5mivw6cqvobyjph37u3la@okz5aoowa6bm>
Date: Mon, 21 Oct 2024 14:08:57 +0300
From: Serge Semin <fancer.lancer@...il.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Jose Abreu <Jose.Abreu@...opsys.com>, netdev@...r.kernel.org, 
	Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH net-next 0/7] net: pcs: xpcs: yet more cleanups

Hi

On Thu, Oct 17, 2024 at 12:52:17PM GMT, Russell King (Oracle) wrote:
> Hi,
> 
> I've found yet more potential for cleanups in the XPCS driver.
> 
> The first patch switches to using generic register definitions.
> 
> Next, there's an overly complex bit of code in xpcs_link_up_1000basex()
> which can be simplified down to a simple if() statement.
> 
> Then, rearrange xpcs_link_up_1000basex() to separate out the warnings
> from the functional bit.
> 
> Next, realising that the functional bit is just the helper function we
> already have and are using in the SGMII version of this function,
> switch over to that.
> 
> We can now see that xpcs_link_up_1000basex() and xpcs_link_up_sgmii()
> are basically functionally identical except for the warnings, so merge
> the two functions.
> 
> Next, xpcs_config_usxgmii() seems misnamed, so rename it to follow the
> established pattern.
> 
> Lastly, "return foo();" where foo is a void function and the function
> being returned from is also void is a weird programming pattern.
> Replace this with something more conventional.
> 
> With these changes, we see yet another reduction in the amount of
> code in this driver.

If you wish this to be tested before merging in, I'll be able to do
that tomorrow or on Wednesday. In anyway I'll get back with the
results after testing the series out.

-Serge(y)

> 
>  drivers/net/pcs/pcs-xpcs.c | 134 ++++++++++++++++++++++-----------------------
>  drivers/net/pcs/pcs-xpcs.h |  12 ----
>  2 files changed, 65 insertions(+), 81 deletions(-)
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
>