Skip to contentSkip to conversation

Micrel Phy - Is there a way to configure the Phy not to do 802.3x flow control?

DISCUSSION Murali Karicheri 3 participants 13 messages Mar 3, 2016 – Mar 18, 2016

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

Hi,

We are using Micrel Phy in one of our board and wondering if we can force the Phy to disable flow control at start. I have a 1G ethernet switch connected to Phy and the phy always enable flow control. I would like to configure the phy not to flow control. Is that possible and if yes, what should I do in the my Ethernet driver to tell the Phy not to enable flow control?

Signature · 2 lines
Murali Karicheri
Linux Kernel, Keystone
maintainer
#1 lore raw
Quoted 7 lines · Murali Karicheri

Hi,

We are using Micrel Phy in one of our board and wondering if we can force the Phy to disable flow control at start. I have a 1G ethernet switch connected to Phy and the phy always enable flow control. I would like to configure the phy not to flow control. Is that possible and if yes, what should I do in the my Ethernet driver to tell the Phy not to enable flow control?

Hi Murali

Have you played with:

ethtool -a|--show-pause devname

ethtool -A|--pause devname [autoneg on|off] [rx on|off] [tx on|off]

Andrew

#3 lore raw
In reply to Murali Karicheri
Quoted 7 lines · Murali Karicheri

Hi,

We are using Micrel Phy in one of our board and wondering if we can force the Phy to disable flow control at start. I have a 1G ethernet switch connected to Phy and the phy always enable flow control. I would like to configure the phy not to flow control. Is that possible and if yes, what should I do in the my Ethernet driver to tell the Phy not to enable flow control?

The PHY is not doing flow control per-se, your pseudo Ethernet MAC in the switch is doing, along with the link partner advertising support for it. You would want to make sure that your PHY device interface (provided that you are using the PHY library) is not starting with Pause advertised, but it could be supported.

As Andrew indicated the proper way to do this is do to use ethtool if you need to this dynamically.

Signature · 1 line
Florian
#2 lore raw
In reply to Andrew Lunn

Andrew,

Thanks for your response!

Quoted 11 lines · Andrew Lunn
Quoted 7 lines · Murali Karicheri

Hi,

We are using Micrel Phy in one of our board and wondering if we can force the Phy to disable flow control at start. I have a 1G ethernet switch connected to Phy and the phy always enable flow control. I would like to configure the phy not to flow control. Is that possible and if yes, what should I do in the my Ethernet driver to tell the Phy not to enable flow control?

Hi Murali

Quoted 8 lines

Have you played with:

ethtool -a|--show-pause devname

ethtool -A|--pause devname [autoneg on|off] [rx on|off] [tx on|off]

Andrew

I will try, but my question is for disabling flow control by default in the phy when Ethernet driver is initialized. How does the driver tells the phy to disable tx/rx flow control. Even if phy advertise flow control capability the Ethernet h/w MAC layer should be capable of doing a Pause. So the driver needs to have a way to disable tx/rx FC when it starts. How this can be achieved? We would like to disable this by default. User will be able to enable it through ethtool command if it desire to have FC for a specific use case. Isn't reasonable?

Signature · 2 lines
Murali Karicheri
Linux Kernel, Keystone
#4 lore raw
In reply to Florian Fainelli
Quoted 9 lines · Florian Fainelli
Quoted 7 lines · Murali Karicheri

Hi,

We are using Micrel Phy in one of our board and wondering if we can force the Phy to disable flow control at start. I have a 1G ethernet switch connected to Phy and the phy always enable flow control. I would like to configure the phy not to flow control. Is that possible and if yes, what should I do in the my Ethernet driver to tell the Phy not to enable flow control?

Quoted 5 lines

The PHY is not doing flow control per-se, your pseudo Ethernet MAC in the switch is doing, along with the link partner advertising support for it. You would want to make sure that your PHY device interface (provided that you are using the PHY library) is not starting with Pause advertised, but it could be supported.

Understood that Phy is just advertise FC. The Micrel phy for 9031 advertise by default FC supported. After negotiation, I see that Phylib provide the link status with parameter pause = 1, asym_pause = 1. How do I tell the Phy not to advertise?

I call following sequence in the Ethernet driver.

of_phy_connect(x,y,hndlr,a,z);
phy_start()

Now in hndlr() I have pause = 1, asym_pause = 1, in phy_device ptr. How can I tell the phy not to advertise initially?

Murali

Quoted 4 lines

As Andrew indicated the proper way to do this is do to use ethtool if you need to this dynamically.

Signature · 2 lines
Murali Karicheri
Linux Kernel, Keystone
#5 lore raw
Quoted 16 lines · Murali Karicheri
Quoted 9 lines · Florian Fainelli
Quoted 7 lines · Murali Karicheri

Hi,

We are using Micrel Phy in one of our board and wondering if we can force the Phy to disable flow control at start. I have a 1G ethernet switch connected to Phy and the phy always enable flow control. I would like to configure the phy not to flow control. Is that possible and if yes, what should I do in the my Ethernet driver to tell the Phy not to enable flow control?

Quoted 5 lines

The PHY is not doing flow control per-se, your pseudo Ethernet MAC in the switch is doing, along with the link partner advertising support for it. You would want to make sure that your PHY device interface (provided that you are using the PHY library) is not starting with Pause advertised, but it could be supported.

Quoted 8 lines

Understood that Phy is just advertise FC. The Micrel phy for 9031 advertise by default FC supported. After negotiation, I see that Phylib provide the link status with parameter pause = 1, asym_pause = 1. How do I tell the Phy not to advertise?

I call following sequence in the Ethernet driver.

of_phy_connect(x,y,hndlr,a,z);

Here you should be able to change phydev->advertising and phydev->supported to mask the ADVERTISED_Pause | ADVERTISED_AsymPause bits and have phy_start() restart with that which should disable pause and asym_pause as seen by your adjust_link handler.

Quoted 4 lines

phy_start()

Now in hndlr() I have pause = 1, asym_pause = 1, in phy_device ptr. How can I tell the phy not to advertise initially?

Signature · 1 line
Florian
#6 lore raw
Quoted 22 lines · Florian Fainelli
Quoted 21 lines · Murali Karicheri
Quoted 9 lines · Florian Fainelli
Quoted 7 lines · Murali Karicheri

Hi,

We are using Micrel Phy in one of our board and wondering if we can force the Phy to disable flow control at start. I have a 1G ethernet switch connected to Phy and the phy always enable flow control. I would like to configure the phy not to flow control. Is that possible and if yes, what should I do in the my Ethernet driver to tell the Phy not to enable flow control?

Quoted 5 lines

The PHY is not doing flow control per-se, your pseudo Ethernet MAC in the switch is doing, along with the link partner advertising support for it. You would want to make sure that your PHY device interface (provided that you are using the PHY library) is not starting with Pause advertised, but it could be supported.

Understood that Phy is just advertise FC. The Micrel phy for 9031 advertise by default FC supported. After negotiation, I see that Phylib provide the link status with parameter pause = 1, asym_pause = 1. How do I tell the Phy not to advertise?

Quoted 9 lines
Quoted 3 lines

I call following sequence in the Ethernet driver.

of_phy_connect(x,y,hndlr,a,z);

Here you should be able to change phydev->advertising and phydev->supported to mask the ADVERTISED_Pause | ADVERTISED_AsymPause bits and have phy_start() restart with that which should disable pause and asym_pause as seen by your adjust_link handler.

Ok. Good point. I will try this. Thanks for your suggestion.

Murali

Quoted 4 lines
Quoted 4 lines

phy_start()

Now in hndlr() I have pause = 1, asym_pause = 1, in phy_device ptr. How can I tell the phy not to advertise initially?

Signature · 2 lines
Murali Karicheri
Linux Kernel, Keystone
#7 lore raw
Quoted 25 lines · Murali Karicheri
Quoted 24 lines · Florian Fainelli
Quoted 23 lines · Murali Karicheri
Quoted 9 lines · Florian Fainelli
Quoted 7 lines · Murali Karicheri

Hi,

We are using Micrel Phy in one of our board and wondering if we can force the Phy to disable flow control at start. I have a 1G ethernet switch connected to Phy and the phy always enable flow control. I would like to configure the phy not to flow control. Is that possible and if yes, what should I do in the my Ethernet driver to tell the Phy not to enable flow control?

Quoted 5 lines

The PHY is not doing flow control per-se, your pseudo Ethernet MAC in the switch is doing, along with the link partner advertising support for it. You would want to make sure that your PHY device interface (provided that you are using the PHY library) is not starting with Pause advertised, but it could be supported.

Understood that Phy is just advertise FC. The Micrel phy for 9031 advertise by default FC supported. After negotiation, I see that Phylib provide the link status with parameter pause = 1, asym_pause = 1. How do I tell the Phy not to advertise?

I call following sequence in the Ethernet driver.

Quoted 9 lines
Quoted 7 lines
Quoted 1 line

of_phy_connect(x,y,hndlr,a,z);

Here you should be able to change phydev->advertising and phydev->supported to mask the ADVERTISED_Pause | ADVERTISED_AsymPause bits and have phy_start() restart with that which should disable pause and asym_pause as seen by your adjust_link handler.

Ok. Good point. I will try this. Thanks for your suggestion.

I made following changes. The phylib still report flow control enabled to the driver. Some bug in the phylib/phydev?

+
+ printk("slave->phy->supported %x, slave->phy->advertising %x\n",
+ slave->phy->supported, slave->phy->advertising);
+ slave->phy->supported &=
+ ~(SUPPORTED_Pause | SUPPORTED_Asym_Pause);
+ slave->phy->advertising = slave->phy->supported;
+ printk("slave->phy->supported %x, slave->phy->advertising %x\n",

+                       slave->phy->supported, slave->phy->advertising);
                phy_start(slave->phy);
+               printk("slave->phy->supported %x, slave->phy->advertising %x\n",
+                       slave->phy->supported, slave->phy->advertising);
                phy_read_status(slave->phy);
[   10.757001] slave->phy->supported 22ff, slave->phy->advertising 22ff
[   10.763354] slave->phy->supported 2ff, slave->phy->advertising 2ff
[   10.769552] slave->phy->supported 2ff, slave->phy->advertising 2ff
[   10.776045] netcp-1.0 2620110.netcp eth0: Link is Down
udhcpc (v1.23.1) started
Sending discover...
Sending discover...
[   14.757280] netcp-1.0 2620110.netcp eth0: Link is Up - 1Gbps/Full - flow control rx/tx

Sending discover...
Sending select for 158.218.103.170...
Lease of 158.218.103.170 obtained, lease time 28800 /etc/udhcpc.d/50default: Adding DNS 192.0.2.2 /etc/udhcpc.d/50default: Adding DNS 192.0.2.3

Quoted 7 lines

Murali

Quoted 4 lines
Quoted 4 lines

phy_start()

Now in hndlr() I have pause = 1, asym_pause = 1, in phy_device ptr. How can I tell the phy not to advertise initially?

Signature · 2 lines
Murali Karicheri
Linux Kernel, Keystone
#8 lore raw
In reply to Murali Karicheri
Quoted 25 lines · Murali Karicheri
Quoted 24 lines · Florian Fainelli
Quoted 23 lines · Murali Karicheri
Quoted 9 lines · Florian Fainelli
Quoted 7 lines · Murali Karicheri

Hi,

We are using Micrel Phy in one of our board and wondering if we can force the Phy to disable flow control at start. I have a 1G ethernet switch connected to Phy and the phy always enable flow control. I would like to configure the phy not to flow control. Is that possible and if yes, what should I do in the my Ethernet driver to tell the Phy not to enable flow control?

Quoted 5 lines

The PHY is not doing flow control per-se, your pseudo Ethernet MAC in the switch is doing, along with the link partner advertising support for it. You would want to make sure that your PHY device interface (provided that you are using the PHY library) is not starting with Pause advertised, but it could be supported.

Understood that Phy is just advertise FC. The Micrel phy for 9031 advertise by default FC supported. After negotiation, I see that Phylib provide the link status with parameter pause = 1, asym_pause = 1. How do I tell the Phy not to advertise?

I call following sequence in the Ethernet driver.

Quoted 9 lines
Quoted 7 lines
Quoted 1 line

of_phy_connect(x,y,hndlr,a,z);

Here you should be able to change phydev->advertising and phydev->supported to mask the ADVERTISED_Pause | ADVERTISED_AsymPause bits and have phy_start() restart with that which should disable pause and asym_pause as seen by your adjust_link handler.

Ok. Good point. I will try this. Thanks for your suggestion.

I had to make following changes to the phy_device.c to allow the phy device report maximum common flow control capability to Ethernet driver through handler. My driver code looks like this.

                slave->phy = of_phy_connect(gbe_intf->ndev,
                                            slave->phy_node,
                                            hndlr, 0,
                                            phy_mode);
                if (!slave->phy) {
                        dev_err(priv->dev, "phy not found on slave %d\n",
                                slave->slave_num);
                        return -ENODEV;
                }
                dev_dbg(priv->dev, "phy found: id is: 0x%s\n",
                        dev_name(&slave->phy->dev));

                slave->phy->supported &=
                                ~(SUPPORTED_Pause | SUPPORTED_Asym_Pause);
                slave->phy->advertising = slave->phy->supported;
                phy_start(slave->phy);
                phy_read_status(slave->phy);

And then in the phy_device.c, I did to get flow control off reported in handler for link status update.

drivers/net/phy/phy_device.c +2 -2
index d551df6..55412ad 100644
@@ -1021,8 +1021,8 @@ int genphy_read_status(struct phy_device *phydev)
                                phydev->duplex = DUPLEX_FULL;
 
                if (phydev->duplex == DUPLEX_FULL) {
-                       phydev->pause = lpa & LPA_PAUSE_CAP ? 1 : 0;
-                       phydev->asym_pause = lpa & LPA_PAUSE_ASYM ? 1 : 0;
+                       phydev->pause = adv & lpa & LPA_PAUSE_CAP ? 1 : 0;
+                       phydev->asym_pause = adv & lpa & LPA_PAUSE_ASYM ? 1 : 0;
 

Could you explain, why the common maximum capability is not reported to the driver as per standard?? Or Am I understood it wrong?

Murali

Quoted 7 lines

Murali

Quoted 4 lines
Quoted 4 lines

phy_start()

Now in hndlr() I have pause = 1, asym_pause = 1, in phy_device ptr. How can I tell the phy not to advertise initially?

Signature · 2 lines
Murali Karicheri
Linux Kernel, Keystone
#9 lore raw
Quoted 68 lines · Murali Karicheri
Quoted 25 lines · Murali Karicheri
Quoted 24 lines · Florian Fainelli
Quoted 23 lines · Murali Karicheri
Quoted 9 lines · Florian Fainelli
Quoted 7 lines · Murali Karicheri

Hi,

We are using Micrel Phy in one of our board and wondering if we can force the Phy to disable flow control at start. I have a 1G ethernet switch connected to Phy and the phy always enable flow control. I would like to configure the phy not to flow control. Is that possible and if yes, what should I do in the my Ethernet driver to tell the Phy not to enable flow control?

Quoted 5 lines

The PHY is not doing flow control per-se, your pseudo Ethernet MAC in the switch is doing, along with the link partner advertising support for it. You would want to make sure that your PHY device interface (provided that you are using the PHY library) is not starting with Pause advertised, but it could be supported.

Understood that Phy is just advertise FC. The Micrel phy for 9031 advertise by default FC supported. After negotiation, I see that Phylib provide the link status with parameter pause = 1, asym_pause = 1. How do I tell the Phy not to advertise?

I call following sequence in the Ethernet driver.

Quoted 9 lines
Quoted 7 lines
Quoted 1 line

of_phy_connect(x,y,hndlr,a,z);

Here you should be able to change phydev->advertising and phydev->supported to mask the ADVERTISED_Pause | ADVERTISED_AsymPause bits and have phy_start() restart with that which should disable pause and asym_pause as seen by your adjust_link handler.

Ok. Good point. I will try this. Thanks for your suggestion.

I had to make following changes to the phy_device.c to allow the phy device report maximum common flow control capability to Ethernet driver through handler. My driver code looks like this.

                slave->phy = of_phy_connect(gbe_intf->ndev,
                                            slave->phy_node,
                                            hndlr, 0,
                                            phy_mode);
                if (!slave->phy) {
                        dev_err(priv->dev, "phy not found on slave %d\n",
                                slave->slave_num);
                        return -ENODEV;
                }
                dev_dbg(priv->dev, "phy found: id is: 0x%s\n",
                        dev_name(&slave->phy->dev));

                slave->phy->supported &=
                                ~(SUPPORTED_Pause | SUPPORTED_Asym_Pause);
                slave->phy->advertising = slave->phy->supported;
                phy_start(slave->phy);
                phy_read_status(slave->phy);

And then in the phy_device.c, I did to get flow control off reported in handler for link status update.

drivers/net/phy/phy_device.c +0 -0
index d551df6..55412ad 100644

drivers/net/phy/phy_device.c int genphy_read_status(struct phy_device *phydev) · line 1025

Quoted 7 lines
drivers/net/phy/phy_device.c +2 -2
@@ -1021,8 +1021,8 @@ int genphy_read_status(struct phy_device *phydev)
                                phydev->duplex = DUPLEX_FULL;
 
                if (phydev->duplex == DUPLEX_FULL) {
-                       phydev->pause = lpa & LPA_PAUSE_CAP ? 1 : 0;
-                       phydev->asym_pause = lpa & LPA_PAUSE_ASYM ? 1 : 0;
+                       phydev->pause = adv & lpa & LPA_PAUSE_CAP ? 1 : 0;
+                       phydev->asym_pause = adv & lpa & LPA_PAUSE_ASYM ? 1 : 0;

What it means before your patch is that flow control is reported to the PHY device if the link partner advertises that, with your patch applied, it is reported only if the link partner and yourself advertise flow control.

You seem to be willing to have phydev->pause and phydev->asym_pause reflect the resolved pause capability, as opposed to the link partner's pause capability, which I am not convinced is correct here, because we need to take into account the user-configured pause configuration as well. Your adjust_link function should be the one deciding whether pause frame advertising and enabling is appropriate based on: locally configured pause settings (enabled, disabled, autoneg) and the link partner's pause capability.

I do agree that the two fields are confusing and poorly documented, and we should probably be consolidating the pause frame behavior in PHYLIB as opposed to letting drivers deal with like e.g: gianfar, bcm63xx_enet, tg3 etc.

Quoted 3 lines

Could you explain, why the common maximum capability is not reported to the driver as per standard?? Or Am I understood it wrong?

I do not understand the question, what is "maximum capability" in that context and what standard are you refering to?

Signature · 1 line
Florian
#10 lore raw
Quoted 96 lines · Florian Fainelli
Quoted 68 lines · Murali Karicheri
Quoted 25 lines · Murali Karicheri
Quoted 24 lines · Florian Fainelli
Quoted 23 lines · Murali Karicheri
Quoted 9 lines · Florian Fainelli
Quoted 7 lines · Murali Karicheri

Hi,

We are using Micrel Phy in one of our board and wondering if we can force the Phy to disable flow control at start. I have a 1G ethernet switch connected to Phy and the phy always enable flow control. I would like to configure the phy not to flow control. Is that possible and if yes, what should I do in the my Ethernet driver to tell the Phy not to enable flow control?

Quoted 5 lines

The PHY is not doing flow control per-se, your pseudo Ethernet MAC in the switch is doing, along with the link partner advertising support for it. You would want to make sure that your PHY device interface (provided that you are using the PHY library) is not starting with Pause advertised, but it could be supported.

Understood that Phy is just advertise FC. The Micrel phy for 9031 advertise by default FC supported. After negotiation, I see that Phylib provide the link status with parameter pause = 1, asym_pause = 1. How do I tell the Phy not to advertise?

I call following sequence in the Ethernet driver.

Quoted 9 lines
Quoted 7 lines
Quoted 1 line

of_phy_connect(x,y,hndlr,a,z);

Here you should be able to change phydev->advertising and phydev->supported to mask the ADVERTISED_Pause | ADVERTISED_AsymPause bits and have phy_start() restart with that which should disable pause and asym_pause as seen by your adjust_link handler.

Ok. Good point. I will try this. Thanks for your suggestion.

I had to make following changes to the phy_device.c to allow the phy device report maximum common flow control capability to Ethernet driver through handler. My driver code looks like this.

                slave->phy = of_phy_connect(gbe_intf->ndev,
                                            slave->phy_node,
                                            hndlr, 0,
                                            phy_mode);
                if (!slave->phy) {
                        dev_err(priv->dev, "phy not found on slave %d\n",
                                slave->slave_num);
                        return -ENODEV;
                }
                dev_dbg(priv->dev, "phy found: id is: 0x%s\n",
                        dev_name(&slave->phy->dev));

                slave->phy->supported &=
                                ~(SUPPORTED_Pause | SUPPORTED_Asym_Pause);
                slave->phy->advertising = slave->phy->supported;
                phy_start(slave->phy);
                phy_read_status(slave->phy);

And then in the phy_device.c, I did to get flow control off reported in handler for link status update.

drivers/net/phy/phy_device.c +0 -0
index d551df6..55412ad 100644

drivers/net/phy/phy_device.c int genphy_read_status(struct phy_device *phydev) · line 1025

Quoted 7 lines
drivers/net/phy/phy_device.c +2 -2
@@ -1021,8 +1021,8 @@ int genphy_read_status(struct phy_device *phydev)
                                phydev->duplex = DUPLEX_FULL;
 
                if (phydev->duplex == DUPLEX_FULL) {
-                       phydev->pause = lpa & LPA_PAUSE_CAP ? 1 : 0;
-                       phydev->asym_pause = lpa & LPA_PAUSE_ASYM ? 1 : 0;
+                       phydev->pause = adv & lpa & LPA_PAUSE_CAP ? 1 : 0;
+                       phydev->asym_pause = adv & lpa & LPA_PAUSE_ASYM ? 1 : 0;

What it means before your patch is that flow control is reported to the PHY device if the link partner advertises that, with your patch applied, it is reported only if the link partner and yourself advertise flow control.

You seem to be willing to have phydev->pause and phydev->asym_pause reflect the resolved pause capability, as opposed to the link partner's pause capability, which I am not convinced is correct here, because we need to take into account the user-configured pause configuration as well. Your adjust_link function should be the one deciding whether pause frame advertising and enabling is appropriate based on: locally configured pause settings (enabled, disabled, autoneg) and the link partner's pause capability.

I do agree that the two fields are confusing and poorly documented, and we should probably be consolidating the pause frame behavior in PHYLIB as opposed to letting drivers deal with like e.g: gianfar, bcm63xx_enet, tg3 etc.

Quoted 1 line
Quoted 6 lines
Quoted 2 lines

Could you explain, why the common maximum capability is not reported to the driver as per standard?? Or Am I understood it wrong?

I do not understand the question, what is "maximum capability" in that context and what standard are you refering to?

I assume the Phylib is responsible for deciding what is the flow control pause and asym pause status to the driver through adjust_link. When driver starts the phy, it also tells its capabilities (for example it can reset some of the capabilities available in the Phy driver. In this particular case, Pause is a feature supported by Micrel phy. I have reset this feature in my driver by resetting this feature bit in the phy_device as suggested earlier in this discussion). So phylib has all knowledge available to disable flow control in this scenario even if LP is capable of flow control. Wondering why every driver has to take this decision again to enable or disable flow control instead of telling the driver what to do.

Looks like Marvel driver (reproduced below) does this logic. I think the 802.3x flow control states, but I don't have access to the standard documentation. However I find the documentation at
http://www.studioreti.it/slide/08_SwFlowContr_E_A.pdf.

Page 17 states the behavior based on Local device & Link partner's capabilities. Probably adjust_link() should tell the outcome in pause and asym_pause so that driver can enable/disable fc. Also user's action to be taken into account as well so that fc can be disabled if desired.

Code from drivers/net/phy/marvel.c

static int marvell_read_status(struct phy_device *phydev) {

        int adv;
        int err;
        int lpa;
        int lpagb;
        int status = 0;

        /* Update the link, but return if there
         * was an error */
        err = genphy_update_link(phydev);
        if (err)
                return err;

        if (AUTONEG_ENABLE == phydev->autoneg) {
                status = phy_read(phydev, MII_M1011_PHY_STATUS);
                if (status < 0)
                        return status;

                lpa = phy_read(phydev, MII_LPA);
                if (lpa < 0)
                        return lpa;

                lpagb = phy_read(phydev, MII_STAT1000);
                if (lpagb < 0)
                        return lpagb;

                adv = phy_read(phydev, MII_ADVERTISE);
                if (adv < 0)
                        return adv;

                phydev->lp_advertising = mii_stat1000_to_ethtool_lpa_t(lpagb) |
                                         mii_lpa_to_ethtool_lpa_t(lpa);

                lpa &= adv;

                if (status & MII_M1011_PHY_STATUS_FULLDUPLEX        int adv;
        int err;
        int lpa;
        int lpagb;
        int status = 0;

        /* Update the link, but return if there
         * was an error */
        err = genphy_update_link(phydev);
        if (err)
                return err;
)
                        phydev->duplex = DUPLEX_FULL;
                else
                        phydev->duplex = DUPLEX_HALF;

                status = status & MII_M1011_PHY_STATUS_SPD_MASK;
                phydev->pause = phydev->asym_pause = 0;

                switch (status) {
                case MII_M1011_PHY_STATUS_1000:
                        phydev->speed = SPEED_1000;
                        break;

                case MII_M1011_PHY_STATUS_100:
                        phydev->speed = SPEED_100;
                        break;

                default:
                        phydev->speed = SPEED_10;

                        break;
                }

                if (phydev->duplex == DUPLEX_FULL) {
                        phydev->pause = lpa & LPA_PAUSE_CAP ? 1 : 0;
                        phydev->asym_pause = lpa & LPA_PAUSE_ASYM ? 1 : 0;
                }
        } else {
                int bmcr = phy_read(phydev, MII_BMCR);

                if (bmcr < 0)
                        return bmcr;

                if (bmcr & BMCR_FULLDPLX)
                        phydev->duplex = DUPLEX_FULL;
                else
                        phydev->duplex = DUPLEX_HALF;

                if (bmcr & BMCR_SPEED1000)
                        phydev->speed = SPEED_1000;
                else if (bmcr & BMCR_SPEED100)
                        phydev->speed = SPEED_100;
                else
                        phydev->speed = SPEED_10;

                phydev->pause = phydev->asym_pause = 0;
                phydev->lp_advertising = 0;
        }

        return 0;
}
Signature · 2 lines
Murali Karicheri
Linux Kernel, Keystone
#11 lore raw
Quoted 103 lines · Murali Karicheri
Quoted 96 lines · Florian Fainelli
Quoted 68 lines · Murali Karicheri
Quoted 25 lines · Murali Karicheri
Quoted 24 lines · Florian Fainelli
Quoted 23 lines · Murali Karicheri
Quoted 9 lines · Florian Fainelli
Quoted 7 lines · Murali Karicheri

Hi,

We are using Micrel Phy in one of our board and wondering if we can force the Phy to disable flow control at start. I have a 1G ethernet switch connected to Phy and the phy always enable flow control. I would like to configure the phy not to flow control. Is that possible and if yes, what should I do in the my Ethernet driver to tell the Phy not to enable flow control?

Quoted 5 lines

The PHY is not doing flow control per-se, your pseudo Ethernet MAC in the switch is doing, along with the link partner advertising support for it. You would want to make sure that your PHY device interface (provided that you are using the PHY library) is not starting with Pause advertised, but it could be supported.

Understood that Phy is just advertise FC. The Micrel phy for 9031 advertise by default FC supported. After negotiation, I see that Phylib provide the link status with parameter pause = 1, asym_pause = 1. How do I tell the Phy not to advertise?

I call following sequence in the Ethernet driver.

Quoted 9 lines
Quoted 7 lines
Quoted 1 line

of_phy_connect(x,y,hndlr,a,z);

Here you should be able to change phydev->advertising and phydev->supported to mask the ADVERTISED_Pause | ADVERTISED_AsymPause bits and have phy_start() restart with that which should disable pause and asym_pause as seen by your adjust_link handler.

Ok. Good point. I will try this. Thanks for your suggestion.

I had to make following changes to the phy_device.c to allow the phy device report maximum common flow control capability to Ethernet driver through handler. My driver code looks like this.

                slave->phy = of_phy_connect(gbe_intf->ndev,
                                            slave->phy_node,
                                            hndlr, 0,
                                            phy_mode);
                if (!slave->phy) {
                        dev_err(priv->dev, "phy not found on slave %d\n",
                                slave->slave_num);
                        return -ENODEV;
                }
                dev_dbg(priv->dev, "phy found: id is: 0x%s\n",
                        dev_name(&slave->phy->dev));

                slave->phy->supported &=
                                ~(SUPPORTED_Pause | SUPPORTED_Asym_Pause);
                slave->phy->advertising = slave->phy->supported;
                phy_start(slave->phy);
                phy_read_status(slave->phy);

And then in the phy_device.c, I did to get flow control off reported in handler for link status update.

drivers/net/phy/phy_device.c +0 -0
index d551df6..55412ad 100644

drivers/net/phy/phy_device.c int genphy_read_status(struct phy_device *phydev) · line 1025

Quoted 7 lines
drivers/net/phy/phy_device.c +2 -2
@@ -1021,8 +1021,8 @@ int genphy_read_status(struct phy_device *phydev)
                                phydev->duplex = DUPLEX_FULL;
 
                if (phydev->duplex == DUPLEX_FULL) {
-                       phydev->pause = lpa & LPA_PAUSE_CAP ? 1 : 0;
-                       phydev->asym_pause = lpa & LPA_PAUSE_ASYM ? 1 : 0;
+                       phydev->pause = adv & lpa & LPA_PAUSE_CAP ? 1 : 0;
+                       phydev->asym_pause = adv & lpa & LPA_PAUSE_ASYM ? 1 : 0;

What it means before your patch is that flow control is reported to the PHY device if the link partner advertises that, with your patch applied, it is reported only if the link partner and yourself advertise flow control.

You seem to be willing to have phydev->pause and phydev->asym_pause reflect the resolved pause capability, as opposed to the link partner's pause capability, which I am not convinced is correct here, because we need to take into account the user-configured pause configuration as well. Your adjust_link function should be the one deciding whether pause frame advertising and enabling is appropriate based on: locally configured pause settings (enabled, disabled, autoneg) and the link partner's pause capability.

I do agree that the two fields are confusing and poorly documented, and we should probably be consolidating the pause frame behavior in PHYLIB as opposed to letting drivers deal with like e.g: gianfar, bcm63xx_enet, tg3 etc.

Quoted 1 line
Quoted 6 lines
Quoted 2 lines

Could you explain, why the common maximum capability is not reported to the driver as per standard?? Or Am I understood it wrong?

I do not understand the question, what is "maximum capability" in that context and what standard are you refering to?

Quoted 11 lines

I assume the Phylib is responsible for deciding what is the flow control pause and asym pause status to the driver through adjust_link. When driver starts the phy, it also tells its capabilities (for example it can reset some of the capabilities available in the Phy driver. In this particular case, Pause is a feature supported by Micrel phy. I have reset this feature in my driver by resetting this feature bit in the phy_device as suggested earlier in this discussion). So phylib has all knowledge available to disable flow control in this scenario even if LP is capable of flow control. Wondering why every driver has to take this decision again to enable or disable flow control instead of telling the driver what to do.

What I meant is if driver tells phy not advertise, then adjust_link should reflect the same. i.e both pause and asym_pause should be off.

Murali

Quoted 114 lines

Looks like Marvel driver (reproduced below) does this logic. I think the 802.3x flow control states, but I don't have access to the standard documentation. However I find the documentation at
http://www.studioreti.it/slide/08_SwFlowContr_E_A.pdf.

Page 17 states the behavior based on Local device & Link partner's capabilities. Probably adjust_link() should tell the outcome in pause and asym_pause so that driver can enable/disable fc. Also user's action to be taken into account as well so that fc can be disabled if desired.

Code from drivers/net/phy/marvel.c

static int marvell_read_status(struct phy_device *phydev) {

        int adv;
        int err;
        int lpa;
        int lpagb;
        int status = 0;

        /* Update the link, but return if there
         * was an error */
        err = genphy_update_link(phydev);
        if (err)
                return err;

        if (AUTONEG_ENABLE == phydev->autoneg) {
                status = phy_read(phydev, MII_M1011_PHY_STATUS);
                if (status < 0)
                        return status;

                lpa = phy_read(phydev, MII_LPA);
                if (lpa < 0)
                        return lpa;

                lpagb = phy_read(phydev, MII_STAT1000);
                if (lpagb < 0)
                        return lpagb;

                adv = phy_read(phydev, MII_ADVERTISE);
                if (adv < 0)
                        return adv;

                phydev->lp_advertising = mii_stat1000_to_ethtool_lpa_t(lpagb) |
                                         mii_lpa_to_ethtool_lpa_t(lpa);

                lpa &= adv;

                if (status & MII_M1011_PHY_STATUS_FULLDUPLEX        int adv;
        int err;
        int lpa;
        int lpagb;
        int status = 0;

        /* Update the link, but return if there
         * was an error */
        err = genphy_update_link(phydev);
        if (err)
                return err;
)
                        phydev->duplex = DUPLEX_FULL;
                else
                        phydev->duplex = DUPLEX_HALF;

                status = status & MII_M1011_PHY_STATUS_SPD_MASK;
                phydev->pause = phydev->asym_pause = 0;

                switch (status) {
                case MII_M1011_PHY_STATUS_1000:
                        phydev->speed = SPEED_1000;
                        break;

                case MII_M1011_PHY_STATUS_100:
                        phydev->speed = SPEED_100;
                        break;

                default:
                        phydev->speed = SPEED_10;

                        break;
                }

                if (phydev->duplex == DUPLEX_FULL) {
                        phydev->pause = lpa & LPA_PAUSE_CAP ? 1 : 0;
                        phydev->asym_pause = lpa & LPA_PAUSE_ASYM ? 1 : 0;
                }
        } else {
                int bmcr = phy_read(phydev, MII_BMCR);

                if (bmcr < 0)
                        return bmcr;

                if (bmcr & BMCR_FULLDPLX)
                        phydev->duplex = DUPLEX_FULL;
                else
                        phydev->duplex = DUPLEX_HALF;

                if (bmcr & BMCR_SPEED1000)
                        phydev->speed = SPEED_1000;
                else if (bmcr & BMCR_SPEED100)
                        phydev->speed = SPEED_100;
                else
                        phydev->speed = SPEED_10;

                phydev->pause = phydev->asym_pause = 0;
                phydev->lp_advertising = 0;
        }

        return 0;
}
Signature · 2 lines
Murali Karicheri
Linux Kernel, Keystone
#12 lore raw
In reply to Murali Karicheri

On March 16, 2016 8:08:14 AM PDT, Murali Karicheri

Quoted 30 lines
Quoted 1 line

What it means before your patch is that flow control is reported to

the

Quoted 1 line

PHY device if the link partner advertises that, with your patch

applied,

Quoted 1 line

it is reported only if the link partner and yourself advertise flow

control.

Quoted 3 lines

You seem to be willing to have phydev->pause and phydev->asym_pause reflect the resolved pause capability, as opposed to the link

partner's

Quoted 1 line

pause capability, which I am not convinced is correct here, because

we

Quoted 2 lines

need to take into account the user-configured pause configuration as well. Your adjust_link function should be the one deciding whether

pause

Quoted 5 lines

frame advertising and enabling is appropriate based on: locally configured pause settings (enabled, disabled, autoneg) and the link partner's pause capability.

I do agree that the two fields are confusing and poorly documented,

and

Quoted 1 line

we should probably be consolidating the pause frame behavior in

PHYLIB

Quoted 1 line

as opposed to letting drivers deal with like e.g: gianfar,

bcm63xx_enet,

Quoted 4 lines

tg3 etc.

Quoted 2 lines

Could you explain, why the common maximum capability is not reported

to the

Quoted 8 lines
Quoted 3 lines
Quoted 1 line

driver as per standard?? Or Am I understood it wrong?

I do not understand the question, what is "maximum capability" in

that

Quoted 2 lines

context and what standard are you refering to?

I assume the Phylib is responsible for deciding what is the flow control pause and asym pause status to the driver through adjust_link.

Not exclusively, the Ethernet MAC driver is responsible for getting the full picture: whether pause frames need to be advertised, enabled locally and supported by the link partner. PHYLIB helps with the later since the former may come from ethtool:: get_pauseparam and need to program MAC specific registers within the adjust_link callback.

Quoted 5 lines

When driver starts the phy, it also tells its capabilities (for example it can reset some of the capabilities available in the Phy driver. In this
particular case, Pause is a feature supported by Micrel phy. I have reset

Quoted 8 lines

this feature in my driver by resetting this feature bit in the phy_device
as suggested earlier in this discussion). So phylib has all knowledge available to disable flow control in this scenario even if LP is capable
of flow control. Wondering why every driver has to take this decision again to enable or disable flow control instead of telling the driver what to do.

PHYLIB still misses the MAC driver decisions like whether pause frames are to be enabled by default through autoneg or manually enforced via an user, which is why it reports what the link partner's pause capability so as to get your driver to be able to make the right decisions here.

Quoted 8 lines

Looks like Marvel driver (reproduced below) does this logic. I think the
802.3x flow control states, but I don't have access to the standard documentation.
However I find the documentation at
http://www.studioreti.it/slide/08_SwFlowContr_E_A.pdf.

Quoted 6 lines

Page 17 states the behavior based on Local device & Link partner's capabilities. Probably adjust_link() should tell the outcome in pause and asym_pause so that driver can enable/disable fc. Also user's action to be taken into account as well so that fc can be disabled if desired.

Code from drivers/net/phy/marvel.c

Well, thanks for poing that piece of code, that seems to be duplicating a bit of what genphy_read_status() is already doing.