net: phy: mediatek-ge-soc: follow netdev LED trigger semantics

Only blink if the link is up on a LED which is programmed to also
indicate link-status.

Otherwise, if both LEDs are in use to indicate different speeds, the
resulting blinking being inverted on LEDs which aren't switched on at
a specific speed is quite counter-intuitive.

Also make sure that state left behind by reset or the bootloader is
recognized correctly including the half-duplex and full-duplex bits as
well as the (unsupported by Linux netdev trigger semantics) link-down
bit.

Fixes: c66937b0f8db ("net: phy: mediatek-ge-soc: support PHY LEDs")
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/drivers/net/phy/mediatek-ge-soc.c b/drivers/net/phy/mediatek-ge-soc.c
index 0f3a1538a8b8ee045953a3c5ff308dc824ea7c0a..f4f9412d0cd7e256f4b2e962dd18974e1120c0fb 100644 (file)
--- a/drivers/net/phy/mediatek-ge-soc.c
+++ b/drivers/net/phy/mediatek-ge-soc.c
@@ -216,6 +216,9 @@
 #define   MTK_PHY_LED_ON_LINK1000              BIT(0)
 #define   MTK_PHY_LED_ON_LINK100               BIT(1)
 #define   MTK_PHY_LED_ON_LINK10                        BIT(2)
+#define   MTK_PHY_LED_ON_LINK                  (MTK_PHY_LED_ON_LINK10 |\
+                                                MTK_PHY_LED_ON_LINK100 |\
+                                                MTK_PHY_LED_ON_LINK1000)
 #define   MTK_PHY_LED_ON_LINKDOWN              BIT(3)
 #define   MTK_PHY_LED_ON_FDX                   BIT(4) /* Full duplex */
 #define   MTK_PHY_LED_ON_HDX                   BIT(5) /* Half duplex */
@@ -231,6 +234,12 @@
 #define   MTK_PHY_LED_BLINK_100RX              BIT(3)
 #define   MTK_PHY_LED_BLINK_10TX               BIT(4)
 #define   MTK_PHY_LED_BLINK_10RX               BIT(5)
+#define   MTK_PHY_LED_BLINK_RX                 (MTK_PHY_LED_BLINK_10RX |\
+                                                MTK_PHY_LED_BLINK_100RX |\
+                                                MTK_PHY_LED_BLINK_1000RX)
+#define   MTK_PHY_LED_BLINK_TX                 (MTK_PHY_LED_BLINK_10TX |\
+                                                MTK_PHY_LED_BLINK_100TX |\
+                                                MTK_PHY_LED_BLINK_1000TX)
 #define   MTK_PHY_LED_BLINK_COLLISION          BIT(6)
 #define   MTK_PHY_LED_BLINK_RX_CRC_ERR         BIT(7)
 #define   MTK_PHY_LED_BLINK_RX_IDLE_ERR                BIT(8)
@@ -1247,11 +1256,9 @@ static int mt798x_phy_led_hw_control_get(struct phy_device *phydev, u8 index,
        if (blink < 0)
                return -EIO;
 
-       if ((on & (MTK_PHY_LED_ON_LINK1000 | MTK_PHY_LED_ON_LINK100 |
-                  MTK_PHY_LED_ON_LINK10)) ||
-           (blink & (MTK_PHY_LED_BLINK_1000RX | MTK_PHY_LED_BLINK_100RX |
-                     MTK_PHY_LED_BLINK_10RX | MTK_PHY_LED_BLINK_1000TX |
-                     MTK_PHY_LED_BLINK_100TX | MTK_PHY_LED_BLINK_10TX)))
+       if ((on & (MTK_PHY_LED_ON_LINK | MTK_PHY_LED_ON_FDX | MTK_PHY_LED_ON_HDX |
+                  MTK_PHY_LED_ON_LINKDOWN)) ||
+           (blink & (MTK_PHY_LED_BLINK_RX | MTK_PHY_LED_BLINK_TX)))
                set_bit(bit_netdev, &priv->led_state);
        else
                clear_bit(bit_netdev, &priv->led_state);
@@ -1269,7 +1276,7 @@ static int mt798x_phy_led_hw_control_get(struct phy_device *phydev, u8 index,
        if (!rules)
                return 0;
 
-       if (on & (MTK_PHY_LED_ON_LINK1000 | MTK_PHY_LED_ON_LINK100 | MTK_PHY_LED_ON_LINK10))
+       if (on & MTK_PHY_LED_ON_LINK)
                *rules |= BIT(TRIGGER_NETDEV_LINK);
 
        if (on & MTK_PHY_LED_ON_LINK10)
@@ -1287,10 +1294,10 @@ static int mt798x_phy_led_hw_control_get(struct phy_device *phydev, u8 index,
        if (on & MTK_PHY_LED_ON_HDX)
                *rules |= BIT(TRIGGER_NETDEV_HALF_DUPLEX);
 
-       if (blink & (MTK_PHY_LED_BLINK_1000RX | MTK_PHY_LED_BLINK_100RX | MTK_PHY_LED_BLINK_10RX))
+       if (blink & MTK_PHY_LED_BLINK_RX)
                *rules |= BIT(TRIGGER_NETDEV_RX);
 
-       if (blink & (MTK_PHY_LED_BLINK_1000TX | MTK_PHY_LED_BLINK_100TX | MTK_PHY_LED_BLINK_10TX))
+       if (blink & MTK_PHY_LED_BLINK_TX)
                *rules |= BIT(TRIGGER_NETDEV_TX);
 
        return 0;
@@ -1323,15 +1330,19 @@ static int mt798x_phy_led_hw_control_set(struct phy_device *phydev, u8 index,
                on |= MTK_PHY_LED_ON_LINK1000;
 
        if (rules & BIT(TRIGGER_NETDEV_RX)) {
-               blink |= MTK_PHY_LED_BLINK_10RX  |
-                        MTK_PHY_LED_BLINK_100RX |
-                        MTK_PHY_LED_BLINK_1000RX;
+               blink |= (on & MTK_PHY_LED_ON_LINK) ?
+                         (((on & MTK_PHY_LED_ON_LINK10) ? MTK_PHY_LED_BLINK_10RX : 0) |
+                          ((on & MTK_PHY_LED_ON_LINK100) ? MTK_PHY_LED_BLINK_100RX : 0) |
+                          ((on & MTK_PHY_LED_ON_LINK1000) ? MTK_PHY_LED_BLINK_1000RX : 0)) :
+                         MTK_PHY_LED_BLINK_RX;
        }
 
        if (rules & BIT(TRIGGER_NETDEV_TX)) {
-               blink |= MTK_PHY_LED_BLINK_10TX  |
-                        MTK_PHY_LED_BLINK_100TX |
-                        MTK_PHY_LED_BLINK_1000TX;
+               blink |= (on & MTK_PHY_LED_ON_LINK) ?
+                         (((on & MTK_PHY_LED_ON_LINK10) ? MTK_PHY_LED_BLINK_10TX : 0) |
+                          ((on & MTK_PHY_LED_ON_LINK100) ? MTK_PHY_LED_BLINK_100TX : 0) |
+                          ((on & MTK_PHY_LED_ON_LINK1000) ? MTK_PHY_LED_BLINK_1000TX : 0)) :
+                         MTK_PHY_LED_BLINK_TX;
        }
 
        if (blink || on)
@@ -1344,9 +1355,7 @@ static int mt798x_phy_led_hw_control_set(struct phy_device *phydev, u8 index,
                                MTK_PHY_LED0_ON_CTRL,
                             MTK_PHY_LED_ON_FDX     |
                             MTK_PHY_LED_ON_HDX     |
-                            MTK_PHY_LED_ON_LINK10  |
-                            MTK_PHY_LED_ON_LINK100 |
-                            MTK_PHY_LED_ON_LINK1000,
+                            MTK_PHY_LED_ON_LINK,
                             on);
 
        if (ret)