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: <20230521102809.i3o55e4nuuy7dtdu@skbuf>
Date:   Sun, 21 May 2023 13:28:09 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Oleksij Rempel <o.rempel@...gutronix.de>
Cc:     Woojung Huh <woojung.huh@...rochip.com>,
        Andrew Lunn <andrew@...n.ch>,
        Arun Ramadoss <arun.ramadoss@...rochip.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Simon Horman <simon.horman@...igine.com>,
        "Russell King (Oracle)" <linux@...linux.org.uk>,
        linux-kernel@...r.kernel.org, Eric Dumazet <edumazet@...gle.com>,
        Paolo Abeni <pabeni@...hat.com>, kernel@...gutronix.de,
        netdev@...r.kernel.org, Jakub Kicinski <kuba@...nel.org>,
        UNGLinuxDriver@...rochip.com,
        "David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH net-next v4 1/2] net: dsa: microchip: ksz8: Make flow
 control, speed, and duplex on CPU port configurable

On Sun, May 21, 2023 at 06:38:41AM +0200, Oleksij Rempel wrote:
> Looks good, I like the idea with "wacky" registers!
> 
> Would you prefer that I start working on adapting your patch set to the
> KSZ8873? Or should I make a review to move forward the existing patch set?
> 
> Just a heads up, I don't have access to the KSZ87xx series switches, so
> I won't be able to test the changes on these models.
> 
> Let me know what you think and how we should proceed.

If we convert to regfields, the entire driver will need to be converted
(all switch families). I'd say make a best effort full conversion, and
if something breaks on the families which you could not test, surely
someone will jump to correct it. Since your KSZ8873 also has wacky registers
(btw, feel free to rename the concept to something more serious), I think
that not a lot can go wrong with a blind conversion as long as it's tested
on other hardware.

BTW, revisiting, I already found a bug in the conversion (patch 2/4):

+	} else if (mii_sel == bitval[P_RMII_SEL]) {
 		interface = PHY_INTERFACE_MODE_RGMII;
 	} else {
+		ret = ksz_regfields_read(dev, port, RF_RGMII_ID_IG_ENABLE, &ig);
+		if (WARN_ON(ret))
+			return PHY_INTERFACE_MODE_NA;
+
+		ret = ksz_regfields_read(dev, port, RF_RGMII_ID_IG_ENABLE, &eg);
						    ~~~~~~~~~~~~~~~~~~~~~
						    should have been RF_RGMII_ID_EG_ENABLE
+		if (WARN_ON(ret))
+			return PHY_INTERFACE_MODE_NA;
+
 		interface = PHY_INTERFACE_MODE_RGMII;
-		if (data8 & P_RGMII_ID_EG_ENABLE)
+		if (eg)
 			interface = PHY_INTERFACE_MODE_RGMII_TXID;
-		if (data8 & P_RGMII_ID_IG_ENABLE) {
+		if (ig) {
 			interface = PHY_INTERFACE_MODE_RGMII_RXID;
-			if (data8 & P_RGMII_ID_EG_ENABLE)
+			if (eg)
 				interface = PHY_INTERFACE_MODE_RGMII_ID;
 		}
 	}

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ