[PATCH RFC net-next] net: phy: intel-xway: remove LED configuration

From: Michael Walle
Date: Thu Mar 02 2023 - 09:17:15 EST

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@xxxxxxxx>
---
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