net: phy: mediatek: fix TX blink masks using the RX bits

[PATCH net] net: phy: mediatek: fix TX blink masks using the RX bits

Posted by Ahmed Naseef 1 month, 4 weeks ago

MTK_GPHY_LED_TX_BLINK_SET and MTK_2P5GPHY_LED_TX_BLINK_SET are built
from the RX blink bits instead of the TX ones, so both TX masks are
identical to their RX counterparts. The TX bits they should be using,
MTK_PHY_LED_BLINK_{10,100,1000,2500}TX, are otherwise only referenced
by the per-speed branch of mtk_phy_led_hw_ctrl_set().

A TX trigger selected without a link trigger therefore programs the RX
blink bits, and the LED blinks on received traffic. The masks are also
used to decode the blink register in mtk_phy_led_hw_ctrl_get(), which
as a result cannot tell the two triggers apart: an RX-only
configuration reads back as RX and TX, and a TX-only configuration
reads back as neither.

Fixes: 7f9c320c98db ("net: phy: mediatek: Move LED helper functions into mtk phy lib")
Cc: stable@vger.kernel.org
Signed-off-by: Ahmed Naseef <naseefkm@gmail.com>
---
 drivers/net/phy/mediatek/mtk.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/mediatek/mtk.h b/drivers/net/phy/mediatek/mtk.h
index 320f76ffa81f..79e09727b81b 100644
--- a/drivers/net/phy/mediatek/mtk.h
+++ b/drivers/net/phy/mediatek/mtk.h
@@ -53,15 +53,15 @@
 #define MTK_GPHY_LED_RX_BLINK_SET		(MTK_PHY_LED_BLINK_1000RX | \
 						 MTK_PHY_LED_BLINK_100RX | \
 						 MTK_PHY_LED_BLINK_10RX)
-#define MTK_GPHY_LED_TX_BLINK_SET		(MTK_PHY_LED_BLINK_1000RX | \
-						 MTK_PHY_LED_BLINK_100RX | \
-						 MTK_PHY_LED_BLINK_10RX)
+#define MTK_GPHY_LED_TX_BLINK_SET		(MTK_PHY_LED_BLINK_1000TX | \
+						 MTK_PHY_LED_BLINK_100TX | \
+						 MTK_PHY_LED_BLINK_10TX)
 
 #define MTK_2P5GPHY_LED_ON_SET			(MTK_PHY_LED_ON_LINK2500 | \
 						 MTK_GPHY_LED_ON_SET)
 #define MTK_2P5GPHY_LED_RX_BLINK_SET		(MTK_PHY_LED_BLINK_2500RX | \
 						 MTK_GPHY_LED_RX_BLINK_SET)
-#define MTK_2P5GPHY_LED_TX_BLINK_SET		(MTK_PHY_LED_BLINK_2500RX | \
+#define MTK_2P5GPHY_LED_TX_BLINK_SET		(MTK_PHY_LED_BLINK_2500TX | \
 						 MTK_GPHY_LED_TX_BLINK_SET)
 
 #define MTK_PHY_LED_STATE_FORCE_ON	0

base-commit: 1a35da325cac4d5bcad76a2aa943408a6f1d9000
-- 
2.34.1

Re: [PATCH net] net: phy: mediatek: fix TX blink masks using the RX bits

Posted by Andrew Lunn 1 month, 4 weeks ago

On Tue, Aug 04, 2026 at 03:35:11PM +0400, Ahmed Naseef wrote:
> MTK_GPHY_LED_TX_BLINK_SET and MTK_2P5GPHY_LED_TX_BLINK_SET are built
> from the RX blink bits instead of the TX ones, so both TX masks are
> identical to their RX counterparts. The TX bits they should be using,
> MTK_PHY_LED_BLINK_{10,100,1000,2500}TX, are otherwise only referenced
> by the per-speed branch of mtk_phy_led_hw_ctrl_set().
> 
> A TX trigger selected without a link trigger therefore programs the RX
> blink bits, and the LED blinks on received traffic. The masks are also
> used to decode the blink register in mtk_phy_led_hw_ctrl_get(), which
> as a result cannot tell the two triggers apart: an RX-only
> configuration reads back as RX and TX, and a TX-only configuration
> reads back as neither.
> 
> Fixes: 7f9c320c98db ("net: phy: mediatek: Move LED helper functions into mtk phy lib")

Was this bug introduced in this commit, or did this commit just move
the code around and it was broken before? The Fixes should go back to
where the bug was added, which could be earlier.

	Andrew

Re: [PATCH net] net: phy: mediatek: fix TX blink masks using the RX bits

Posted by Ahmed Naseef 1 month, 4 weeks ago

On Tue, Aug 04, 2026 at 08:11:21PM +0200, Andrew Lunn wrote:
> On Tue, Aug 04, 2026 at 03:35:11PM +0400, Ahmed Naseef wrote:
> > MTK_GPHY_LED_TX_BLINK_SET and MTK_2P5GPHY_LED_TX_BLINK_SET are built
> > from the RX blink bits instead of the TX ones, so both TX masks are
> > identical to their RX counterparts. The TX bits they should be using,
> > MTK_PHY_LED_BLINK_{10,100,1000,2500}TX, are otherwise only referenced
> > by the per-speed branch of mtk_phy_led_hw_ctrl_set().
> > 
> > A TX trigger selected without a link trigger therefore programs the RX
> > blink bits, and the LED blinks on received traffic. The masks are also
> > used to decode the blink register in mtk_phy_led_hw_ctrl_get(), which
> > as a result cannot tell the two triggers apart: an RX-only
> > configuration reads back as RX and TX, and a TX-only configuration
> > reads back as neither.
> > 
> > Fixes: 7f9c320c98db ("net: phy: mediatek: Move LED helper functions into mtk phy lib")
> 
> Was this bug introduced in this commit, or did this commit just move
> the code around and it was broken before? The Fixes should go back to
> where the bug was added, which could be earlier.

It was introduced by that commit. Before it, mtk-ge-soc.c correctly had:

  #define   MTK_PHY_LED_BLINK_TX      (MTK_PHY_LED_BLINK_10TX |\
                                       MTK_PHY_LED_BLINK_100TX |\
                                       MTK_PHY_LED_BLINK_1000TX)

which 7f9c320c98db removed, adding to mtk.h in the same commit:

  #define MTK_GPHY_LED_TX_BLINK_SET   (MTK_PHY_LED_BLINK_1000RX | \
                                       MTK_PHY_LED_BLINK_100RX | \
                                       MTK_PHY_LED_BLINK_10RX)


Thanks,
Ahmed

> 
> 	Andrew

Re: [PATCH net] net: phy: mediatek: fix TX blink masks using the RX bits

Posted by Andrew Lunn 1 month, 4 weeks ago

On Tue, Aug 04, 2026 at 10:32:38PM +0400, Ahmed Naseef wrote:
> On Tue, Aug 04, 2026 at 08:11:21PM +0200, Andrew Lunn wrote:
> > On Tue, Aug 04, 2026 at 03:35:11PM +0400, Ahmed Naseef wrote:
> > > MTK_GPHY_LED_TX_BLINK_SET and MTK_2P5GPHY_LED_TX_BLINK_SET are built
> > > from the RX blink bits instead of the TX ones, so both TX masks are
> > > identical to their RX counterparts. The TX bits they should be using,
> > > MTK_PHY_LED_BLINK_{10,100,1000,2500}TX, are otherwise only referenced
> > > by the per-speed branch of mtk_phy_led_hw_ctrl_set().
> > > 
> > > A TX trigger selected without a link trigger therefore programs the RX
> > > blink bits, and the LED blinks on received traffic. The masks are also
> > > used to decode the blink register in mtk_phy_led_hw_ctrl_get(), which
> > > as a result cannot tell the two triggers apart: an RX-only
> > > configuration reads back as RX and TX, and a TX-only configuration
> > > reads back as neither.
> > > 
> > > Fixes: 7f9c320c98db ("net: phy: mediatek: Move LED helper functions into mtk phy lib")
> > 
> > Was this bug introduced in this commit, or did this commit just move
> > the code around and it was broken before? The Fixes should go back to
> > where the bug was added, which could be earlier.
> 
> It was introduced by that commit. Before it, mtk-ge-soc.c correctly had:
> 
>   #define   MTK_PHY_LED_BLINK_TX      (MTK_PHY_LED_BLINK_10TX |\
>                                        MTK_PHY_LED_BLINK_100TX |\
>                                        MTK_PHY_LED_BLINK_1000TX)

Thanks for checking.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew