C45 Phys and PHY_FORCING state

C45 Phys and PHY_FORCING state

[Jose Abreu]

[-- Attachment #1: Type: text/plain, Size: 516 bytes --]

Hello Andrew and Florian,

Currently I have a 10G C45 phy that is fixed at 10G link. This
version does not support auto negotiation so I'm turning off the
feature in phydev struct field. I found out that when I do this
phylib is not composing C45 frames and is instead using C22. This
is due to call to genphy_udpate_link() which doesn't work on my
phy because it doesn't support C22.

If I apply attached patch then things work perfectly fine. Can
you please review it ?

Thanks and Best Regards,
Jose Miguel Abreu

[-- Attachment #2: 0005-net-phy-Use-C45-Helpers-when-forcing-PHY.patch --]
[-- Type: text/x-patch, Size: 1109 bytes --]

>From cccf07f4d7335cbc36f3856da6d368cb01570760 Mon Sep 17 00:00:00 2001
Message-Id: <cccf07f4d7335cbc36f3856da6d368cb01570760.1539949939.git.joabreu@synopsys.com>
In-Reply-To: <cover.1539949939.git.joabreu@synopsys.com>
References: <cover.1539949939.git.joabreu@synopsys.com>
From: Jose Abreu <joabreu@synopsys.com>
Date: Thu, 18 Oct 2018 17:36:21 +0200
Subject: [PATCH 5/8] net: phy: Use C45 Helpers when forcing PHY

If PHY is in force state and we have a C45 phy we need to use the
standard C45 helpers and not the C22 ones.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
---
 drivers/net/phy/phy.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 1ee25877c4d1..28ed957bc0f6 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -1007,7 +1007,11 @@ void phy_state_machine(struct work_struct *work)
 		}
 		break;
 	case PHY_FORCING:
-		err = genphy_update_link(phydev);
+		if (phydev->is_c45)
+			err = gen10g_read_status(phydev);
+		else
+			err = genphy_update_link(phydev);
+
 		if (err)
 			break;
 
-- 
2.7.4

Re: C45 Phys and PHY_FORCING state

[Florian Fainelli]

On 10/19/2018 05:02 AM, Jose Abreu wrote:
> Hello Andrew and Florian,
> 
> Currently I have a 10G C45 phy that is fixed at 10G link. This
> version does not support auto negotiation so I'm turning off the
> feature in phydev struct field. I found out that when I do this
> phylib is not composing C45 frames and is instead using C22. This
> is due to call to genphy_udpate_link() which doesn't work on my
> phy because it doesn't support C22.
> 
> If I apply attached patch then things work perfectly fine. Can
> you please review it ?

Looks reasonable, I could not find other functions in the state machine
that were not already abstracting the clause type, or letting a driver
callback be called. Can you submit this as a formal patch against
net-next (and not attached, but inline)?

I would suggest creating a helper, e.g: phy_update_link() that way
everything is well namespaced and clear within the state machine itself.
-- 
Florian