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-next>] [day] [month] [year] [list]
Message-Id: <20230302141651.377261-1-michael@walle.cc>
Date:   Thu,  2 Mar 2023 15:16:51 +0100
From:   Michael Walle <michael@...le.cc>
To:     Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        "David S . Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Michael Walle <michael@...le.cc>
Subject: [PATCH RFC net-next] net: phy: intel-xway: remove LED configuration

For this PHY, the LEDs can either be configured through an attached
EEPROM or if not available, the PHY offers sane default modes. Right now,
the driver will configure to a mode just suitable for one configuration
(although there is a bold claim that "most boards have just one LED").
I'd argue, that as long as there is no configuration through other means
(like device tree), the driver shouldn't configure anything LED related
so that the PHY is using either the modes configured by the EEPROM or
the power-on defaults.

Signed-off-by: Michael Walle <michael@...le.cc>
---
I know there is ongoing work on the device tree, but even then, my
argument holds, if there is no config in the device tree, the driver
shouldn't just use "any" configuration when there are means by the
hardware to configure the LEDs.

Not just as an RFC because netdev is closed, but also to get other
opinions. Not to be applied.
---
 drivers/net/phy/intel-xway.c | 149 -----------------------------------
 1 file changed, 149 deletions(-)

diff --git a/drivers/net/phy/intel-xway.c b/drivers/net/phy/intel-xway.c
index 3c032868ef04..4428721238a7 100644
--- a/drivers/net/phy/intel-xway.c
+++ b/drivers/net/phy/intel-xway.c
@@ -18,17 +18,6 @@
 #define XWAY_MDIO_MIICTRL_RXSKEW_MASK	GENMASK(14, 12)
 #define XWAY_MDIO_MIICTRL_TXSKEW_MASK	GENMASK(10, 8)
 
-/* bit 15:12 are reserved */
-#define XWAY_MDIO_LED_LED3_EN		BIT(11)	/* Enable the integrated function of LED3 */
-#define XWAY_MDIO_LED_LED2_EN		BIT(10)	/* Enable the integrated function of LED2 */
-#define XWAY_MDIO_LED_LED1_EN		BIT(9)	/* Enable the integrated function of LED1 */
-#define XWAY_MDIO_LED_LED0_EN		BIT(8)	/* Enable the integrated function of LED0 */
-/* bit 7:4 are reserved */
-#define XWAY_MDIO_LED_LED3_DA		BIT(3)	/* Direct Access to LED3 */
-#define XWAY_MDIO_LED_LED2_DA		BIT(2)	/* Direct Access to LED2 */
-#define XWAY_MDIO_LED_LED1_DA		BIT(1)	/* Direct Access to LED1 */
-#define XWAY_MDIO_LED_LED0_DA		BIT(0)	/* Direct Access to LED0 */
-
 #define XWAY_MDIO_INIT_WOL		BIT(15)	/* Wake-On-LAN */
 #define XWAY_MDIO_INIT_MSRE		BIT(14)
 #define XWAY_MDIO_INIT_NPRX		BIT(13)
@@ -46,111 +35,6 @@
 
 #define ADVERTISED_MPD			BIT(10)	/* Multi-port device */
 
-/* LED Configuration */
-#define XWAY_MMD_LEDCH			0x01E0
-/* Inverse of SCAN Function */
-#define  XWAY_MMD_LEDCH_NACS_NONE	0x0000
-#define  XWAY_MMD_LEDCH_NACS_LINK	0x0001
-#define  XWAY_MMD_LEDCH_NACS_PDOWN	0x0002
-#define  XWAY_MMD_LEDCH_NACS_EEE	0x0003
-#define  XWAY_MMD_LEDCH_NACS_ANEG	0x0004
-#define  XWAY_MMD_LEDCH_NACS_ABIST	0x0005
-#define  XWAY_MMD_LEDCH_NACS_CDIAG	0x0006
-#define  XWAY_MMD_LEDCH_NACS_TEST	0x0007
-/* Slow Blink Frequency */
-#define  XWAY_MMD_LEDCH_SBF_F02HZ	0x0000
-#define  XWAY_MMD_LEDCH_SBF_F04HZ	0x0010
-#define  XWAY_MMD_LEDCH_SBF_F08HZ	0x0020
-#define  XWAY_MMD_LEDCH_SBF_F16HZ	0x0030
-/* Fast Blink Frequency */
-#define  XWAY_MMD_LEDCH_FBF_F02HZ	0x0000
-#define  XWAY_MMD_LEDCH_FBF_F04HZ	0x0040
-#define  XWAY_MMD_LEDCH_FBF_F08HZ	0x0080
-#define  XWAY_MMD_LEDCH_FBF_F16HZ	0x00C0
-/* LED Configuration */
-#define XWAY_MMD_LEDCL			0x01E1
-/* Complex Blinking Configuration */
-#define  XWAY_MMD_LEDCH_CBLINK_NONE	0x0000
-#define  XWAY_MMD_LEDCH_CBLINK_LINK	0x0001
-#define  XWAY_MMD_LEDCH_CBLINK_PDOWN	0x0002
-#define  XWAY_MMD_LEDCH_CBLINK_EEE	0x0003
-#define  XWAY_MMD_LEDCH_CBLINK_ANEG	0x0004
-#define  XWAY_MMD_LEDCH_CBLINK_ABIST	0x0005
-#define  XWAY_MMD_LEDCH_CBLINK_CDIAG	0x0006
-#define  XWAY_MMD_LEDCH_CBLINK_TEST	0x0007
-/* Complex SCAN Configuration */
-#define  XWAY_MMD_LEDCH_SCAN_NONE	0x0000
-#define  XWAY_MMD_LEDCH_SCAN_LINK	0x0010
-#define  XWAY_MMD_LEDCH_SCAN_PDOWN	0x0020
-#define  XWAY_MMD_LEDCH_SCAN_EEE	0x0030
-#define  XWAY_MMD_LEDCH_SCAN_ANEG	0x0040
-#define  XWAY_MMD_LEDCH_SCAN_ABIST	0x0050
-#define  XWAY_MMD_LEDCH_SCAN_CDIAG	0x0060
-#define  XWAY_MMD_LEDCH_SCAN_TEST	0x0070
-/* Configuration for LED Pin x */
-#define XWAY_MMD_LED0H			0x01E2
-/* Fast Blinking Configuration */
-#define  XWAY_MMD_LEDxH_BLINKF_MASK	0x000F
-#define  XWAY_MMD_LEDxH_BLINKF_NONE	0x0000
-#define  XWAY_MMD_LEDxH_BLINKF_LINK10	0x0001
-#define  XWAY_MMD_LEDxH_BLINKF_LINK100	0x0002
-#define  XWAY_MMD_LEDxH_BLINKF_LINK10X	0x0003
-#define  XWAY_MMD_LEDxH_BLINKF_LINK1000	0x0004
-#define  XWAY_MMD_LEDxH_BLINKF_LINK10_0	0x0005
-#define  XWAY_MMD_LEDxH_BLINKF_LINK100X	0x0006
-#define  XWAY_MMD_LEDxH_BLINKF_LINK10XX	0x0007
-#define  XWAY_MMD_LEDxH_BLINKF_PDOWN	0x0008
-#define  XWAY_MMD_LEDxH_BLINKF_EEE	0x0009
-#define  XWAY_MMD_LEDxH_BLINKF_ANEG	0x000A
-#define  XWAY_MMD_LEDxH_BLINKF_ABIST	0x000B
-#define  XWAY_MMD_LEDxH_BLINKF_CDIAG	0x000C
-/* Constant On Configuration */
-#define  XWAY_MMD_LEDxH_CON_MASK	0x00F0
-#define  XWAY_MMD_LEDxH_CON_NONE	0x0000
-#define  XWAY_MMD_LEDxH_CON_LINK10	0x0010
-#define  XWAY_MMD_LEDxH_CON_LINK100	0x0020
-#define  XWAY_MMD_LEDxH_CON_LINK10X	0x0030
-#define  XWAY_MMD_LEDxH_CON_LINK1000	0x0040
-#define  XWAY_MMD_LEDxH_CON_LINK10_0	0x0050
-#define  XWAY_MMD_LEDxH_CON_LINK100X	0x0060
-#define  XWAY_MMD_LEDxH_CON_LINK10XX	0x0070
-#define  XWAY_MMD_LEDxH_CON_PDOWN	0x0080
-#define  XWAY_MMD_LEDxH_CON_EEE		0x0090
-#define  XWAY_MMD_LEDxH_CON_ANEG	0x00A0
-#define  XWAY_MMD_LEDxH_CON_ABIST	0x00B0
-#define  XWAY_MMD_LEDxH_CON_CDIAG	0x00C0
-#define  XWAY_MMD_LEDxH_CON_COPPER	0x00D0
-#define  XWAY_MMD_LEDxH_CON_FIBER	0x00E0
-/* Configuration for LED Pin x */
-#define XWAY_MMD_LED0L			0x01E3
-/* Pulsing Configuration */
-#define  XWAY_MMD_LEDxL_PULSE_MASK	0x000F
-#define  XWAY_MMD_LEDxL_PULSE_NONE	0x0000
-#define  XWAY_MMD_LEDxL_PULSE_TXACT	0x0001
-#define  XWAY_MMD_LEDxL_PULSE_RXACT	0x0002
-#define  XWAY_MMD_LEDxL_PULSE_COL	0x0004
-/* Slow Blinking Configuration */
-#define  XWAY_MMD_LEDxL_BLINKS_MASK	0x00F0
-#define  XWAY_MMD_LEDxL_BLINKS_NONE	0x0000
-#define  XWAY_MMD_LEDxL_BLINKS_LINK10	0x0010
-#define  XWAY_MMD_LEDxL_BLINKS_LINK100	0x0020
-#define  XWAY_MMD_LEDxL_BLINKS_LINK10X	0x0030
-#define  XWAY_MMD_LEDxL_BLINKS_LINK1000	0x0040
-#define  XWAY_MMD_LEDxL_BLINKS_LINK10_0	0x0050
-#define  XWAY_MMD_LEDxL_BLINKS_LINK100X	0x0060
-#define  XWAY_MMD_LEDxL_BLINKS_LINK10XX	0x0070
-#define  XWAY_MMD_LEDxL_BLINKS_PDOWN	0x0080
-#define  XWAY_MMD_LEDxL_BLINKS_EEE	0x0090
-#define  XWAY_MMD_LEDxL_BLINKS_ANEG	0x00A0
-#define  XWAY_MMD_LEDxL_BLINKS_ABIST	0x00B0
-#define  XWAY_MMD_LEDxL_BLINKS_CDIAG	0x00C0
-#define XWAY_MMD_LED1H			0x01E4
-#define XWAY_MMD_LED1L			0x01E5
-#define XWAY_MMD_LED2H			0x01E6
-#define XWAY_MMD_LED2L			0x01E7
-#define XWAY_MMD_LED3H			0x01E8
-#define XWAY_MMD_LED3L			0x01E9
-
 #define PHY_ID_PHY11G_1_3		0x030260D1
 #define PHY_ID_PHY22F_1_3		0x030260E1
 #define PHY_ID_PHY11G_1_4		0xD565A400
@@ -243,39 +127,6 @@ static int xway_gphy_config_init(struct phy_device *phydev)
 	/* Clear all pending interrupts */
 	phy_read(phydev, XWAY_MDIO_ISTAT);
 
-	/* Ensure that integrated led function is enabled for all leds */
-	err = phy_write(phydev, XWAY_MDIO_LED,
-			XWAY_MDIO_LED_LED0_EN |
-			XWAY_MDIO_LED_LED1_EN |
-			XWAY_MDIO_LED_LED2_EN |
-			XWAY_MDIO_LED_LED3_EN);
-	if (err)
-		return err;
-
-	phy_write_mmd(phydev, MDIO_MMD_VEND2, XWAY_MMD_LEDCH,
-		      XWAY_MMD_LEDCH_NACS_NONE |
-		      XWAY_MMD_LEDCH_SBF_F02HZ |
-		      XWAY_MMD_LEDCH_FBF_F16HZ);
-	phy_write_mmd(phydev, MDIO_MMD_VEND2, XWAY_MMD_LEDCL,
-		      XWAY_MMD_LEDCH_CBLINK_NONE |
-		      XWAY_MMD_LEDCH_SCAN_NONE);
-
-	/**
-	 * In most cases only one LED is connected to this phy, so
-	 * configure them all to constant on and pulse mode. LED3 is
-	 * only available in some packages, leave it in its reset
-	 * configuration.
-	 */
-	ledxh = XWAY_MMD_LEDxH_BLINKF_NONE | XWAY_MMD_LEDxH_CON_LINK10XX;
-	ledxl = XWAY_MMD_LEDxL_PULSE_TXACT | XWAY_MMD_LEDxL_PULSE_RXACT |
-		XWAY_MMD_LEDxL_BLINKS_NONE;
-	phy_write_mmd(phydev, MDIO_MMD_VEND2, XWAY_MMD_LED0H, ledxh);
-	phy_write_mmd(phydev, MDIO_MMD_VEND2, XWAY_MMD_LED0L, ledxl);
-	phy_write_mmd(phydev, MDIO_MMD_VEND2, XWAY_MMD_LED1H, ledxh);
-	phy_write_mmd(phydev, MDIO_MMD_VEND2, XWAY_MMD_LED1L, ledxl);
-	phy_write_mmd(phydev, MDIO_MMD_VEND2, XWAY_MMD_LED2H, ledxh);
-	phy_write_mmd(phydev, MDIO_MMD_VEND2, XWAY_MMD_LED2L, ledxl);
-
 	err = xway_gphy_rgmii_init(phydev);
 	if (err)
 		return err;
-- 
2.30.2

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ