[PATCH v3] net: mdio: mux-meson-gxl: set reversed bit when using internal phy

[PATCH v3] net: mdio: mux-meson-gxl: set reversed bit when using internal phy

[Da Xue]

This bit is necessary to receive packets from the internal PHY.
Without this bit set, no activity occurs on the interface.

Normally u-boot sets this bit, but if u-boot is compiled without
net support, the interface will be up but without any activity.

The vendor SDK sets this bit along with the PHY_ID bits.

Fixes: 9a24e1ff4326 ("net: mdio: add amlogic gxl mdio mux support");
Signed-off-by: Da Xue <da@libre.computer>
---
Changes since v2:
* Rename REG2_RESERVED_28 to REG2_REVERSED

Link to v2:
https://patchwork.kernel.org/project/linux-amlogic/patch/20250331074420.3443748-1-christianshewitt@gmail.com/
---
 drivers/net/mdio/mdio-mux-meson-gxl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mdio/mdio-mux-meson-gxl.c b/drivers/net/mdio/mdio-mux-meson-gxl.c
index 00c66240136b..3dd12a8c8b03 100644
--- a/drivers/net/mdio/mdio-mux-meson-gxl.c
+++ b/drivers/net/mdio/mdio-mux-meson-gxl.c
@@ -17,6 +17,7 @@
 #define  REG2_LEDACT		GENMASK(23, 22)
 #define  REG2_LEDLINK		GENMASK(25, 24)
 #define  REG2_DIV4SEL		BIT(27)
+#define  REG2_REVERSED		BIT(28)
 #define  REG2_ADCBYPASS		BIT(30)
 #define  REG2_CLKINSEL		BIT(31)
 #define ETH_REG3		0x4
@@ -65,7 +66,7 @@ static void gxl_enable_internal_mdio(struct gxl_mdio_mux *priv)
 	 * The only constraint is that it must match the one in
 	 * drivers/net/phy/meson-gxl.c to properly match the PHY.
 	 */
-	writel(FIELD_PREP(REG2_PHYID, EPHY_GXL_ID),
+	writel(REG2_REVERSED | FIELD_PREP(REG2_PHYID, EPHY_GXL_ID),
 	       priv->regs + ETH_REG2);
 
 	/* Enable the internal phy */
-- 
2.39.5

Re: [PATCH v3] net: mdio: mux-meson-gxl: set reversed bit when using internal phy

[Simon Horman]

On Fri, Apr 25, 2025 at 03:20:09PM -0400, Da Xue wrote:
> This bit is necessary to receive packets from the internal PHY.
> Without this bit set, no activity occurs on the interface.
> 
> Normally u-boot sets this bit, but if u-boot is compiled without
> net support, the interface will be up but without any activity.
> 
> The vendor SDK sets this bit along with the PHY_ID bits.

I'd like to clarify that:
Without this patch the writel the patch is modifying will clear the PHY_ID bit.
But despite that the system works if at some point (uboot) set the PHY_ID bit?

> 
> Fixes: 9a24e1ff4326 ("net: mdio: add amlogic gxl mdio mux support");

I don't think you need to resend because of this,
but the correct syntax is as follows. (No trailing ';'.)

Fixes: 9a24e1ff4326 ("net: mdio: add amlogic gxl mdio mux support")

> Signed-off-by: Da Xue <da@libre.computer>
> ---
> Changes since v2:
> * Rename REG2_RESERVED_28 to REG2_REVERSED
> 
> Link to v2:
> https://patchwork.kernel.org/project/linux-amlogic/patch/20250331074420.3443748-1-christianshewitt@gmail.com/

...

Re: [PATCH v3] net: mdio: mux-meson-gxl: set reversed bit when using internal phy

[Da Xue]

On Mon, Apr 28, 2025 at 2:13 PM Simon Horman <horms@kernel.org> wrote:
>
> On Fri, Apr 25, 2025 at 03:20:09PM -0400, Da Xue wrote:
> > This bit is necessary to receive packets from the internal PHY.
> > Without this bit set, no activity occurs on the interface.
> >
> > Normally u-boot sets this bit, but if u-boot is compiled without
> > net support, the interface will be up but without any activity.
> >
> > The vendor SDK sets this bit along with the PHY_ID bits.
>
> I'd like to clarify that:
> Without this patch the writel the patch is modifying will clear the PHY_ID bit.
> But despite that the system works if at some point (uboot) set the PHY_ID bit?

Correct. If this is set once, it will work until the IP is powered
down or reset.
If u-boot does not set it, Linux will not set it and the IP will not work.
If u-boot does set it, the IP will not work after suspend-resume since
the IP is reset.
Thus, we need to set it on the Linux side when bringing up the interface.

>
> >
> > Fixes: 9a24e1ff4326 ("net: mdio: add amlogic gxl mdio mux support");
>
> I don't think you need to resend because of this,
> but the correct syntax is as follows. (No trailing ';'.)

Great and thanks! I just set up git send-email so I'm a little rough
around the edges.

>
> Fixes: 9a24e1ff4326 ("net: mdio: add amlogic gxl mdio mux support")
>
> > Signed-off-by: Da Xue <da@libre.computer>
> > ---
> > Changes since v2:
> > * Rename REG2_RESERVED_28 to REG2_REVERSED
> >
> > Link to v2:
> > https://patchwork.kernel.org/project/linux-amlogic/patch/20250331074420.3443748-1-christianshewitt@gmail.com/
>
> ...
>
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v3] net: mdio: mux-meson-gxl: set reversed bit when using internal phy

[Jakub Kicinski]

On Mon, 28 Apr 2025 20:44:14 -0400 Da Xue wrote:
> > On Fri, Apr 25, 2025 at 03:20:09PM -0400, Da Xue wrote:  
> > > This bit is necessary to receive packets from the internal PHY.
> > > Without this bit set, no activity occurs on the interface.
> > >
> > > Normally u-boot sets this bit, but if u-boot is compiled without
> > > net support, the interface will be up but without any activity.
> > >
> > > The vendor SDK sets this bit along with the PHY_ID bits.  
> >
> > I'd like to clarify that:
> > Without this patch the writel the patch is modifying will clear the PHY_ID bit.
> > But despite that the system works if at some point (uboot) set the PHY_ID bit?  
> 
> Correct. If this is set once, it will work until the IP is powered
> down or reset.
> If u-boot does not set it, Linux will not set it and the IP will not work.
> If u-boot does set it, the IP will not work after suspend-resume since
> the IP is reset.
> Thus, we need to set it on the Linux side when bringing up the interface.

Added to the commit message when applying, thank you both!

Re: [PATCH v3] net: mdio: mux-meson-gxl: set reversed bit when using internal phy

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 25 Apr 2025 15:20:09 -0400 you wrote:
> This bit is necessary to receive packets from the internal PHY.
> Without this bit set, no activity occurs on the interface.
> 
> Normally u-boot sets this bit, but if u-boot is compiled without
> net support, the interface will be up but without any activity.
> 
> The vendor SDK sets this bit along with the PHY_ID bits.
> 
> [...]

Here is the summary with links:
  - [v3] net: mdio: mux-meson-gxl: set reversed bit when using internal phy
    https://git.kernel.org/netdev/net/c/b23285e93bef

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html