netdev - Re: [RFC PATCH] net: dsa: microchip: delete dead code

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: <20200722143953.GA1339445@lunn.ch>
Date:   Wed, 22 Jul 2020 16:39:53 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Helmut Grohne <helmut.grohne@...enta.de>
Cc:     Woojung Huh <woojung.huh@...rochip.com>,
        Microchip Linux Driver Support <UNGLinuxDriver@...rochip.com>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org
Subject: Re: [RFC PATCH] net: dsa: microchip: delete dead code

On Tue, Jul 21, 2020 at 10:33:01AM +0200, Helmut Grohne wrote:
> None of the removed assignments is ever read back and never influences
> logic.
> 
> Signed-off-by: Helmut Grohne <helmut.grohne@...enta.de>

Hi Helmut

This patch probably is correct. But it is not obviously correct,
because there are so many changes at once. Please could you break it
up.

> @@ -31,10 +31,7 @@ struct ksz_port {
>  	struct phy_device phydev;
>  
>  	u32 on:1;			/* port is not disabled by hardware */
> -	u32 phy:1;			/* port has a PHY */
>  	u32 fiber:1;			/* port is fiber */
> -	u32 sgmii:1;			/* port is SGMII */
> -	u32 force:1;
>  	u32 read:1;			/* read MIB counters in background */
>  	u32 freeze:1;			/* MIB counter freeze is enabled */
>  
> @@ -71,9 +68,7 @@ struct ksz_device {
>  	int reg_mib_cnt;
>  	int mib_cnt;
>  	int mib_port_cnt;
> -	int last_port;			/* ports after that not used */
>  	phy_interface_t interface;
> -	u32 regs_size;
>  	bool phy_errata_9477;
>  	bool synclko_125;
>  
> @@ -84,14 +79,9 @@ struct ksz_device {
>  	unsigned long mib_read_interval;
>  	u16 br_member;
>  	u16 member;
> -	u16 live_ports;
> -	u16 on_ports;			/* ports enabled by DSA */
> -	u16 rx_ports;
> -	u16 tx_ports;
>  	u16 mirror_rx;
>  	u16 mirror_tx;
>  	u32 features;			/* chip specific features */
> -	u32 overrides;			/* chip functions set by user */
>  	u16 host_mask;
>  	u16 port_mask;

So at minimum, break it up into 3 patches, one per structure.  I would
even go further.

Small patches are easier to review. And if something does break, a git
bisect gives you more information about what change broke it.

Thanks
	Andrew