| Message ID | 1392999584-29836-1-git-send-email-cristian.bercaru@freescale.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
On Fri, 2014-02-21 at 18:19 +0200, cristian.bercaru@freescale.com wrote: > From: Cristian Bercaru <cristian.bercaru@freescale.com> > > Enable the advertisment of symmetric and asymmetric PAUSE frame > capabilities on all Vitesse PHY drivers. I don't think this makes sense. So far as I know, pause frame support depends only on the MAC, not the PHY. I realise there are already other PHY drivers setting these flags, but why should they be added to every PHY driver individually when any autonegotiating PHY can advertise pause capability? Ben. > Signed-off-by: Cristian Bercaru <cristian.bercaru@freescale.com> > --- > drivers/net/phy/vitesse.c | 21 ++++++++++++++------- > 1 file changed, 14 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/phy/vitesse.c b/drivers/net/phy/vitesse.c > index 14372c6..3124b31 100644 > --- a/drivers/net/phy/vitesse.c > +++ b/drivers/net/phy/vitesse.c > @@ -227,7 +227,8 @@ static struct phy_driver vsc82xx_driver[] = { > .phy_id = PHY_ID_VSC8234, > .name = "Vitesse VSC8234", > .phy_id_mask = 0x000ffff0, > - .features = PHY_GBIT_FEATURES, > + .features = PHY_GBIT_FEATURES | > + SUPPORTED_Pause | SUPPORTED_Asym_Pause, > .flags = PHY_HAS_INTERRUPT, > .config_init = &vsc824x_config_init, > .config_aneg = &vsc82x4_config_aneg, > @@ -239,7 +240,8 @@ static struct phy_driver vsc82xx_driver[] = { > .phy_id = PHY_ID_VSC8244, > .name = "Vitesse VSC8244", > .phy_id_mask = 0x000fffc0, > - .features = PHY_GBIT_FEATURES, > + .features = PHY_GBIT_FEATURES | > + SUPPORTED_Pause | SUPPORTED_Asym_Pause, > .flags = PHY_HAS_INTERRUPT, > .config_init = &vsc824x_config_init, > .config_aneg = &vsc82x4_config_aneg, > @@ -251,7 +253,8 @@ static struct phy_driver vsc82xx_driver[] = { > .phy_id = PHY_ID_VSC8514, > .name = "Vitesse VSC8514", > .phy_id_mask = 0x000ffff0, > - .features = PHY_GBIT_FEATURES, > + .features = PHY_GBIT_FEATURES | > + SUPPORTED_Pause | SUPPORTED_Asym_Pause, > .flags = PHY_HAS_INTERRUPT, > .config_init = &vsc824x_config_init, > .config_aneg = &vsc82x4_config_aneg, > @@ -263,7 +266,8 @@ static struct phy_driver vsc82xx_driver[] = { > .phy_id = PHY_ID_VSC8574, > .name = "Vitesse VSC8574", > .phy_id_mask = 0x000ffff0, > - .features = PHY_GBIT_FEATURES, > + .features = PHY_GBIT_FEATURES | > + SUPPORTED_Pause | SUPPORTED_Asym_Pause, > .flags = PHY_HAS_INTERRUPT, > .config_init = &vsc824x_config_init, > .config_aneg = &vsc82x4_config_aneg, > @@ -275,7 +279,8 @@ static struct phy_driver vsc82xx_driver[] = { > .phy_id = PHY_ID_VSC8662, > .name = "Vitesse VSC8662", > .phy_id_mask = 0x000ffff0, > - .features = PHY_GBIT_FEATURES, > + .features = PHY_GBIT_FEATURES | > + SUPPORTED_Pause | SUPPORTED_Asym_Pause, > .flags = PHY_HAS_INTERRUPT, > .config_init = &vsc824x_config_init, > .config_aneg = &vsc82x4_config_aneg, > @@ -288,7 +293,8 @@ static struct phy_driver vsc82xx_driver[] = { > .phy_id = PHY_ID_VSC8221, > .phy_id_mask = 0x000ffff0, > .name = "Vitesse VSC8221", > - .features = PHY_GBIT_FEATURES, > + .features = PHY_GBIT_FEATURES | > + SUPPORTED_Pause | SUPPORTED_Asym_Pause, > .flags = PHY_HAS_INTERRUPT, > .config_init = &vsc8221_config_init, > .config_aneg = &genphy_config_aneg, > @@ -301,7 +307,8 @@ static struct phy_driver vsc82xx_driver[] = { > .phy_id = PHY_ID_VSC8211, > .phy_id_mask = 0x000ffff0, > .name = "Vitesse VSC8211", > - .features = PHY_GBIT_FEATURES, > + .features = PHY_GBIT_FEATURES | > + SUPPORTED_Pause | SUPPORTED_Asym_Pause, > .flags = PHY_HAS_INTERRUPT, > .config_init = &vsc8221_config_init, > .config_aneg = &genphy_config_aneg,
Le samedi 22 février 2014, 13:38:56 Ben Hutchings a écrit : > On Fri, 2014-02-21 at 18:19 +0200, cristian.bercaru@freescale.com wrote: > > From: Cristian Bercaru <cristian.bercaru@freescale.com> > > > > Enable the advertisment of symmetric and asymmetric PAUSE frame > > capabilities on all Vitesse PHY drivers. > > I don't think this makes sense. So far as I know, pause frame support > depends only on the MAC, not the PHY. Agreed. > > I realise there are already other PHY drivers setting these flags, but > why should they be added to every PHY driver individually when any > autonegotiating PHY can advertise pause capability? It looks like this might be abusing a PHY driver features to ensure that the initial value fed to phydev->supported will make us advertise the pause capabilities. This should be set by the Ethernet MAC driver prior to phy_start(), not by the PHY driver. > > Ben. > > > Signed-off-by: Cristian Bercaru <cristian.bercaru@freescale.com> > > --- > > > > drivers/net/phy/vitesse.c | 21 ++++++++++++++------- > > 1 file changed, 14 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/net/phy/vitesse.c b/drivers/net/phy/vitesse.c > > index 14372c6..3124b31 100644 > > --- a/drivers/net/phy/vitesse.c > > +++ b/drivers/net/phy/vitesse.c > > @@ -227,7 +227,8 @@ static struct phy_driver vsc82xx_driver[] = { > > > > .phy_id = PHY_ID_VSC8234, > > .name = "Vitesse VSC8234", > > .phy_id_mask = 0x000ffff0, > > > > - .features = PHY_GBIT_FEATURES, > > + .features = PHY_GBIT_FEATURES | > > + SUPPORTED_Pause | SUPPORTED_Asym_Pause, > > > > .flags = PHY_HAS_INTERRUPT, > > .config_init = &vsc824x_config_init, > > .config_aneg = &vsc82x4_config_aneg, > > > > @@ -239,7 +240,8 @@ static struct phy_driver vsc82xx_driver[] = { > > > > .phy_id = PHY_ID_VSC8244, > > .name = "Vitesse VSC8244", > > .phy_id_mask = 0x000fffc0, > > > > - .features = PHY_GBIT_FEATURES, > > + .features = PHY_GBIT_FEATURES | > > + SUPPORTED_Pause | SUPPORTED_Asym_Pause, > > > > .flags = PHY_HAS_INTERRUPT, > > .config_init = &vsc824x_config_init, > > .config_aneg = &vsc82x4_config_aneg, > > > > @@ -251,7 +253,8 @@ static struct phy_driver vsc82xx_driver[] = { > > > > .phy_id = PHY_ID_VSC8514, > > .name = "Vitesse VSC8514", > > .phy_id_mask = 0x000ffff0, > > > > - .features = PHY_GBIT_FEATURES, > > + .features = PHY_GBIT_FEATURES | > > + SUPPORTED_Pause | SUPPORTED_Asym_Pause, > > > > .flags = PHY_HAS_INTERRUPT, > > .config_init = &vsc824x_config_init, > > .config_aneg = &vsc82x4_config_aneg, > > > > @@ -263,7 +266,8 @@ static struct phy_driver vsc82xx_driver[] = { > > > > .phy_id = PHY_ID_VSC8574, > > .name = "Vitesse VSC8574", > > .phy_id_mask = 0x000ffff0, > > > > - .features = PHY_GBIT_FEATURES, > > + .features = PHY_GBIT_FEATURES | > > + SUPPORTED_Pause | SUPPORTED_Asym_Pause, > > > > .flags = PHY_HAS_INTERRUPT, > > .config_init = &vsc824x_config_init, > > .config_aneg = &vsc82x4_config_aneg, > > > > @@ -275,7 +279,8 @@ static struct phy_driver vsc82xx_driver[] = { > > > > .phy_id = PHY_ID_VSC8662, > > .name = "Vitesse VSC8662", > > .phy_id_mask = 0x000ffff0, > > > > - .features = PHY_GBIT_FEATURES, > > + .features = PHY_GBIT_FEATURES | > > + SUPPORTED_Pause | SUPPORTED_Asym_Pause, > > > > .flags = PHY_HAS_INTERRUPT, > > .config_init = &vsc824x_config_init, > > .config_aneg = &vsc82x4_config_aneg, > > > > @@ -288,7 +293,8 @@ static struct phy_driver vsc82xx_driver[] = { > > > > .phy_id = PHY_ID_VSC8221, > > .phy_id_mask = 0x000ffff0, > > .name = "Vitesse VSC8221", > > > > - .features = PHY_GBIT_FEATURES, > > + .features = PHY_GBIT_FEATURES | > > + SUPPORTED_Pause | SUPPORTED_Asym_Pause, > > > > .flags = PHY_HAS_INTERRUPT, > > .config_init = &vsc8221_config_init, > > .config_aneg = &genphy_config_aneg, > > > > @@ -301,7 +307,8 @@ static struct phy_driver vsc82xx_driver[] = { > > > > .phy_id = PHY_ID_VSC8211, > > .phy_id_mask = 0x000ffff0, > > .name = "Vitesse VSC8211", > > > > - .features = PHY_GBIT_FEATURES, > > + .features = PHY_GBIT_FEATURES | > > + SUPPORTED_Pause | SUPPORTED_Asym_Pause, > > > > .flags = PHY_HAS_INTERRUPT, > > .config_init = &vsc8221_config_init, > > .config_aneg = &genphy_config_aneg,
diff --git a/drivers/net/phy/vitesse.c b/drivers/net/phy/vitesse.c index 14372c6..3124b31 100644 --- a/drivers/net/phy/vitesse.c +++ b/drivers/net/phy/vitesse.c @@ -227,7 +227,8 @@ static struct phy_driver vsc82xx_driver[] = { .phy_id = PHY_ID_VSC8234, .name = "Vitesse VSC8234", .phy_id_mask = 0x000ffff0, - .features = PHY_GBIT_FEATURES, + .features = PHY_GBIT_FEATURES | + SUPPORTED_Pause | SUPPORTED_Asym_Pause, .flags = PHY_HAS_INTERRUPT, .config_init = &vsc824x_config_init, .config_aneg = &vsc82x4_config_aneg, @@ -239,7 +240,8 @@ static struct phy_driver vsc82xx_driver[] = { .phy_id = PHY_ID_VSC8244, .name = "Vitesse VSC8244", .phy_id_mask = 0x000fffc0, - .features = PHY_GBIT_FEATURES, + .features = PHY_GBIT_FEATURES | + SUPPORTED_Pause | SUPPORTED_Asym_Pause, .flags = PHY_HAS_INTERRUPT, .config_init = &vsc824x_config_init, .config_aneg = &vsc82x4_config_aneg, @@ -251,7 +253,8 @@ static struct phy_driver vsc82xx_driver[] = { .phy_id = PHY_ID_VSC8514, .name = "Vitesse VSC8514", .phy_id_mask = 0x000ffff0, - .features = PHY_GBIT_FEATURES, + .features = PHY_GBIT_FEATURES | + SUPPORTED_Pause | SUPPORTED_Asym_Pause, .flags = PHY_HAS_INTERRUPT, .config_init = &vsc824x_config_init, .config_aneg = &vsc82x4_config_aneg, @@ -263,7 +266,8 @@ static struct phy_driver vsc82xx_driver[] = { .phy_id = PHY_ID_VSC8574, .name = "Vitesse VSC8574", .phy_id_mask = 0x000ffff0, - .features = PHY_GBIT_FEATURES, + .features = PHY_GBIT_FEATURES | + SUPPORTED_Pause | SUPPORTED_Asym_Pause, .flags = PHY_HAS_INTERRUPT, .config_init = &vsc824x_config_init, .config_aneg = &vsc82x4_config_aneg, @@ -275,7 +279,8 @@ static struct phy_driver vsc82xx_driver[] = { .phy_id = PHY_ID_VSC8662, .name = "Vitesse VSC8662", .phy_id_mask = 0x000ffff0, - .features = PHY_GBIT_FEATURES, + .features = PHY_GBIT_FEATURES | + SUPPORTED_Pause | SUPPORTED_Asym_Pause, .flags = PHY_HAS_INTERRUPT, .config_init = &vsc824x_config_init, .config_aneg = &vsc82x4_config_aneg, @@ -288,7 +293,8 @@ static struct phy_driver vsc82xx_driver[] = { .phy_id = PHY_ID_VSC8221, .phy_id_mask = 0x000ffff0, .name = "Vitesse VSC8221", - .features = PHY_GBIT_FEATURES, + .features = PHY_GBIT_FEATURES | + SUPPORTED_Pause | SUPPORTED_Asym_Pause, .flags = PHY_HAS_INTERRUPT, .config_init = &vsc8221_config_init, .config_aneg = &genphy_config_aneg, @@ -301,7 +307,8 @@ static struct phy_driver vsc82xx_driver[] = { .phy_id = PHY_ID_VSC8211, .phy_id_mask = 0x000ffff0, .name = "Vitesse VSC8211", - .features = PHY_GBIT_FEATURES, + .features = PHY_GBIT_FEATURES | + SUPPORTED_Pause | SUPPORTED_Asym_Pause, .flags = PHY_HAS_INTERRUPT, .config_init = &vsc8221_config_init, .config_aneg = &genphy_config_aneg,