drivers/net/phy/mscc/mscc.h | 2 ++ drivers/net/phy/mscc/mscc_main.c | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+)
Enable MAC SerDes autonegotiation to distinguish between
1000BASE-X, SGMII and QSGMII MAC.
Signed-off-by: Raag Jadav <raagjadav@gmail.com>
---
drivers/net/phy/mscc/mscc.h | 2 ++
drivers/net/phy/mscc/mscc_main.c | 24 ++++++++++++++++++++++++
2 files changed, 26 insertions(+)
diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h
index a50235f..366db14 100644
--- a/drivers/net/phy/mscc/mscc.h
+++ b/drivers/net/phy/mscc/mscc.h
@@ -195,6 +195,8 @@ enum rgmii_clock_delay {
#define MSCC_PHY_EXTENDED_INT_MS_EGR BIT(9)
/* Extended Page 3 Registers */
+#define MSCC_PHY_SERDES_PCS_CTRL 16
+#define MSCC_PHY_SERDES_ANEG BIT(7)
#define MSCC_PHY_SERDES_TX_VALID_CNT 21
#define MSCC_PHY_SERDES_TX_CRC_ERR_CNT 22
#define MSCC_PHY_SERDES_RX_VALID_CNT 28
diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
index ebfeeb3..6db43a5 100644
--- a/drivers/net/phy/mscc/mscc_main.c
+++ b/drivers/net/phy/mscc/mscc_main.c
@@ -1685,6 +1685,25 @@ static int vsc8574_config_host_serdes(struct phy_device *phydev)
PROC_CMD_RST_CONF_PORT | PROC_CMD_FIBER_1000BASE_X);
}
+static int vsc85xx_config_inband_aneg(struct phy_device *phydev, bool enabled)
+{
+ int rc;
+ u16 reg_val = 0;
+
+ if (enabled)
+ reg_val = MSCC_PHY_SERDES_ANEG;
+
+ mutex_lock(&phydev->lock);
+
+ rc = phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_3,
+ MSCC_PHY_SERDES_PCS_CTRL, MSCC_PHY_SERDES_ANEG,
+ reg_val);
+
+ mutex_unlock(&phydev->lock);
+
+ return rc;
+}
+
static int vsc8584_config_init(struct phy_device *phydev)
{
struct vsc8531_private *vsc8531 = phydev->priv;
@@ -1772,6 +1791,11 @@ static int vsc8584_config_init(struct phy_device *phydev)
VSC8572_RGMII_TX_DELAY_MASK);
if (ret)
return ret;
+ } else {
+ /* Enable clause 37 */
+ ret = vsc85xx_config_inband_aneg(phydev, true);
+ if (ret)
+ return ret;
}
ret = genphy_soft_reset(phydev);
--
2.7.4
On Sat, Feb 05, 2022 at 12:14:52PM +0530, Raag Jadav wrote: > Enable MAC SerDes autonegotiation to distinguish between > 1000BASE-X, SGMII and QSGMII MAC. How does autoneg help you here? It just tells you about duplex, pause etc. It does not indicate 1000BaseX, SGMII etc. The PHY should be using whatever mode it was passed in phydev->interface, which the MAC sets when it calls the connection function. If the PHY dynamically changes its host side mode as a result of what that line side is doing, it should also change phydev->interface. However, as far as i can see, the mscc does not do this. So i don't understand this commit message. Andrew
On Sat, Feb 05, 2022 at 03:57:49PM +0100, Andrew Lunn wrote: > On Sat, Feb 05, 2022 at 12:14:52PM +0530, Raag Jadav wrote: > > Enable MAC SerDes autonegotiation to distinguish between > > 1000BASE-X, SGMII and QSGMII MAC. > > How does autoneg help you here? It just tells you about duplex, pause > etc. It does not indicate 1000BaseX, SGMII etc. The PHY should be > using whatever mode it was passed in phydev->interface, which the MAC > sets when it calls the connection function. If the PHY dynamically > changes its host side mode as a result of what that line side is > doing, it should also change phydev->interface. However, as far as i > can see, the mscc does not do this. > Once the PHY auto-negotiates parameters such as speed and duplex mode with its link partner over the copper link as per IEEE 802.3 Clause 27, the link partner’s capabilities are then transferred by PHY to MAC over 1000BASE-X or SGMII link using the auto-negotiation functionality defined in IEEE 802.3z Clause 37. So any dynamic change in link partner’s capabilities over the copper link can break MAC to PHY communication if MAC SerDes autonegotiation is disabled even on active MAC interface link. Is this understanding correct? > So i don't understand this commit message. > Will send out a v2 with updated commit message on confirmation. Cheers, Raag > Andrew
Hi All,
On 05/02/22 12:14, Raag Jadav wrote:
> Enable MAC SerDes autonegotiation to distinguish between
> 1000BASE-X, SGMII and QSGMII MAC.
>
> Signed-off-by: Raag Jadav <raagjadav@gmail.com>
> ---
> drivers/net/phy/mscc/mscc.h | 2 ++
> drivers/net/phy/mscc/mscc_main.c | 24 ++++++++++++++++++++++++
> 2 files changed, 26 insertions(+)
>
> diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h
> index a50235f..366db14 100644
> --- a/drivers/net/phy/mscc/mscc.h
> +++ b/drivers/net/phy/mscc/mscc.h
> @@ -195,6 +195,8 @@ enum rgmii_clock_delay {
> #define MSCC_PHY_EXTENDED_INT_MS_EGR BIT(9)
>
> /* Extended Page 3 Registers */
> +#define MSCC_PHY_SERDES_PCS_CTRL 16
> +#define MSCC_PHY_SERDES_ANEG BIT(7)
> #define MSCC_PHY_SERDES_TX_VALID_CNT 21
> #define MSCC_PHY_SERDES_TX_CRC_ERR_CNT 22
> #define MSCC_PHY_SERDES_RX_VALID_CNT 28
> diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
> index ebfeeb3..6db43a5 100644
> --- a/drivers/net/phy/mscc/mscc_main.c
> +++ b/drivers/net/phy/mscc/mscc_main.c
> @@ -1685,6 +1685,25 @@ static int vsc8574_config_host_serdes(struct phy_device *phydev)
> PROC_CMD_RST_CONF_PORT | PROC_CMD_FIBER_1000BASE_X);
> }
>
> +static int vsc85xx_config_inband_aneg(struct phy_device *phydev, bool enabled)
> +{
> + int rc;
> + u16 reg_val = 0;
> +
> + if (enabled)
> + reg_val = MSCC_PHY_SERDES_ANEG;
> +
> + mutex_lock(&phydev->lock);
> +
> + rc = phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_3,
> + MSCC_PHY_SERDES_PCS_CTRL, MSCC_PHY_SERDES_ANEG,
> + reg_val);
> +
> + mutex_unlock(&phydev->lock);
> +
> + return rc;
> +}
> +
> static int vsc8584_config_init(struct phy_device *phydev)
> {
> struct vsc8531_private *vsc8531 = phydev->priv;
> @@ -1772,6 +1791,11 @@ static int vsc8584_config_init(struct phy_device *phydev)
> VSC8572_RGMII_TX_DELAY_MASK);
> if (ret)
> return ret;
> + } else {
> + /* Enable clause 37 */
> + ret = vsc85xx_config_inband_aneg(phydev, true);
> + if (ret)
> + return ret;
> }
>
> ret = genphy_soft_reset(phydev);
The same auto-negotiation configuration is also required for VSC8514.
The following patch is required to get Ethernet working with the Quad
port Ethernet Add-On card (QSGMII mode) connected to Texas Instruments
J7 common processor board.
Let me know if I should send it as a separate patch.
Thanks and Regards,
Siddharth Vadapalli.
8<------------------SNIP----------------------------
From 2ab92251ba7a09bc97476cef6c760beefb0d3cae Mon Sep 17 00:00:00 2001
From: Siddharth Vadapalli <s-vadapalli@ti.com>
Date: Thu, 17 Feb 2022 15:45:20 +0530
Subject: [PATCH] net: phy: mscc: Add auto-negotiation feature to VSC8514
Auto-negotiation is currently enabled for VSC8584. It is also required
for VSC8514. Invoke the vsc85xx_config_inband_aneg() function from the
vsc8514_config_init() function present in mscc_main.c to start the
auto-negotiation process. This is required to get Ethernet working with
the Quad port Ethernet Add-On card (QSGMII mode) connected to Texas
Instruments J7 common processor board.
Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
drivers/net/phy/mscc/mscc_main.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/phy/mscc/mscc_main.c
b/drivers/net/phy/mscc/mscc_main.c
index 6db43a5c3b5e..b9a5662e7934 100644
--- a/drivers/net/phy/mscc/mscc_main.c
+++ b/drivers/net/phy/mscc/mscc_main.c
@@ -2119,6 +2119,11 @@ static int vsc8514_config_init(struct phy_device
*phydev)
ret = genphy_soft_reset(phydev);
+ if (ret)
+ return ret;
+
+ ret = vsc85xx_config_inband_aneg(phydev, true);
+
if (ret)
return ret;
Sorry for the late comment on this patch.
On Sat, Feb 05, 2022 at 12:14:52PM +0530, Raag Jadav wrote:
> +static int vsc85xx_config_inband_aneg(struct phy_device *phydev, bool enabled)
> +{
> + int rc;
> + u16 reg_val = 0;
> +
> + if (enabled)
> + reg_val = MSCC_PHY_SERDES_ANEG;
> +
> + mutex_lock(&phydev->lock);
> +
> + rc = phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_3,
> + MSCC_PHY_SERDES_PCS_CTRL, MSCC_PHY_SERDES_ANEG,
> + reg_val);
> +
> + mutex_unlock(&phydev->lock);
What is the reason for the locking here?
phy_modify_paged() itself is safe due to the MDIO bus lock, so you
shouldn't need locking around it.
Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
On Thu, Feb 24, 2022 at 10:48:57AM +0000, Russell King (Oracle) wrote:
> Sorry for the late comment on this patch.
>
> On Sat, Feb 05, 2022 at 12:14:52PM +0530, Raag Jadav wrote:
> > +static int vsc85xx_config_inband_aneg(struct phy_device *phydev, bool enabled)
> > +{
> > + int rc;
> > + u16 reg_val = 0;
> > +
> > + if (enabled)
> > + reg_val = MSCC_PHY_SERDES_ANEG;
> > +
> > + mutex_lock(&phydev->lock);
> > +
> > + rc = phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_3,
> > + MSCC_PHY_SERDES_PCS_CTRL, MSCC_PHY_SERDES_ANEG,
> > + reg_val);
> > +
> > + mutex_unlock(&phydev->lock);
>
> What is the reason for the locking here?
>
> phy_modify_paged() itself is safe due to the MDIO bus lock, so you
> shouldn't need locking around it.
>
True.
My initial thought was to have serialized access at PHY level,
as we have multiple ports to work with.
But I guess MDIO bus lock could do the job as well.
Will fix it in v2 if required.
I've gone through Vladimir's patches and they look more promising
than this approach.
Let me know if I could be of any help.
Cheers,
Raag
> Thanks.
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Hi Raag,
On 26/02/22 12:53, Raag Jadav wrote:
> On Thu, Feb 24, 2022 at 10:48:57AM +0000, Russell King (Oracle) wrote:
>> Sorry for the late comment on this patch.
>>
>> On Sat, Feb 05, 2022 at 12:14:52PM +0530, Raag Jadav wrote:
>>> +static int vsc85xx_config_inband_aneg(struct phy_device *phydev, bool enabled)
>>> +{
>>> + int rc;
>>> + u16 reg_val = 0;
>>> +
>>> + if (enabled)
>>> + reg_val = MSCC_PHY_SERDES_ANEG;
>>> +
>>> + mutex_lock(&phydev->lock);
>>> +
>>> + rc = phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_3,
>>> + MSCC_PHY_SERDES_PCS_CTRL, MSCC_PHY_SERDES_ANEG,
>>> + reg_val);
>>> +
>>> + mutex_unlock(&phydev->lock);
>>
>> What is the reason for the locking here?
>>
>> phy_modify_paged() itself is safe due to the MDIO bus lock, so you
>> shouldn't need locking around it.
>>
>
> True.
>
> My initial thought was to have serialized access at PHY level,
> as we have multiple ports to work with.
> But I guess MDIO bus lock could do the job as well.
>
> Will fix it in v2 if required.
Could you please let me know if you plan to post the v2 patch?
The autonegotiation feature is also required for VSC8514, and has to be invoked
in vsc8514_config_init(). Let me know if you need my help for this.
Regards,
Siddharth
On Thu, Mar 24, 2022 at 03:36:02PM +0530, Siddharth Vadapalli wrote:
> Hi Raag,
>
> On 26/02/22 12:53, Raag Jadav wrote:
> > On Thu, Feb 24, 2022 at 10:48:57AM +0000, Russell King (Oracle) wrote:
> >> Sorry for the late comment on this patch.
> >>
> >> On Sat, Feb 05, 2022 at 12:14:52PM +0530, Raag Jadav wrote:
> >>> +static int vsc85xx_config_inband_aneg(struct phy_device *phydev, bool enabled)
> >>> +{
> >>> + int rc;
> >>> + u16 reg_val = 0;
> >>> +
> >>> + if (enabled)
> >>> + reg_val = MSCC_PHY_SERDES_ANEG;
> >>> +
> >>> + mutex_lock(&phydev->lock);
> >>> +
> >>> + rc = phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_3,
> >>> + MSCC_PHY_SERDES_PCS_CTRL, MSCC_PHY_SERDES_ANEG,
> >>> + reg_val);
> >>> +
> >>> + mutex_unlock(&phydev->lock);
> >>
> >> What is the reason for the locking here?
> >>
> >> phy_modify_paged() itself is safe due to the MDIO bus lock, so you
> >> shouldn't need locking around it.
> >>
> >
> > True.
> >
> > My initial thought was to have serialized access at PHY level,
> > as we have multiple ports to work with.
> > But I guess MDIO bus lock could do the job as well.
> >
> > Will fix it in v2 if required.
>
> Could you please let me know if you plan to post the v2 patch?
>
> The autonegotiation feature is also required for VSC8514, and has to be invoked
> in vsc8514_config_init(). Let me know if you need my help for this.
>
Maybe this is what you're looking for.
https://www.spinics.net/lists/netdev/msg768517.html
Cheers,
Raag
> Regards,
> Siddharth
Hi Raag,
On 27/03/22 14:00, Raag Jadav wrote:
> On Thu, Mar 24, 2022 at 03:36:02PM +0530, Siddharth Vadapalli wrote:
>> Hi Raag,
>>
>> On 26/02/22 12:53, Raag Jadav wrote:
>>> On Thu, Feb 24, 2022 at 10:48:57AM +0000, Russell King (Oracle) wrote:
>>>> Sorry for the late comment on this patch.
>>>>
>>>> On Sat, Feb 05, 2022 at 12:14:52PM +0530, Raag Jadav wrote:
>>>>> +static int vsc85xx_config_inband_aneg(struct phy_device *phydev, bool enabled)
>>>>> +{
>>>>> + int rc;
>>>>> + u16 reg_val = 0;
>>>>> +
>>>>> + if (enabled)
>>>>> + reg_val = MSCC_PHY_SERDES_ANEG;
>>>>> +
>>>>> + mutex_lock(&phydev->lock);
>>>>> +
>>>>> + rc = phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_3,
>>>>> + MSCC_PHY_SERDES_PCS_CTRL, MSCC_PHY_SERDES_ANEG,
>>>>> + reg_val);
>>>>> +
>>>>> + mutex_unlock(&phydev->lock);
>>>>
>>>> What is the reason for the locking here?
>>>>
>>>> phy_modify_paged() itself is safe due to the MDIO bus lock, so you
>>>> shouldn't need locking around it.
>>>>
>>>
>>> True.
>>>
>>> My initial thought was to have serialized access at PHY level,
>>> as we have multiple ports to work with.
>>> But I guess MDIO bus lock could do the job as well.
>>>
>>> Will fix it in v2 if required.
>>
>> Could you please let me know if you plan to post the v2 patch?
>>
>> The autonegotiation feature is also required for VSC8514, and has to be invoked
>> in vsc8514_config_init(). Let me know if you need my help for this.
>>
>
> Maybe this is what you're looking for.
> https://www.spinics.net/lists/netdev/msg768517.html
Thank you for pointing me to the thread. I will follow up regarding the progress
of the autonegotiation feature on that thread.
Regards,
Siddharth
© 2016 - 2026 Red Hat, Inc.