* [PATCH net-next 0/2] net: phy: improve pause handling
@ 2019-04-29 20:12 Heiner Kallweit
2019-04-29 20:13 ` [PATCH net-next 1/2] " Heiner Kallweit
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Heiner Kallweit @ 2019-04-29 20:12 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev
Based on a recent discussion with Andrew that's an attempt to improve
several aspects of how phylib handles sym/asym pause.
Heiner Kallweit (2):
net: phy: improve pause handling
net: phy: improve phy_set_sym_pause and phy_set_asym_pause
drivers/net/phy/fixed_phy.c | 2 +-
drivers/net/phy/phy-core.c | 2 +-
drivers/net/phy/phy_device.c | 64 ++++++++++++++++++++++++++++--------
include/linux/phy.h | 1 +
4 files changed, 54 insertions(+), 15 deletions(-)
--
2.21.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net-next 1/2] net: phy: improve pause handling
2019-04-29 20:12 [PATCH net-next 0/2] net: phy: improve pause handling Heiner Kallweit
@ 2019-04-29 20:13 ` Heiner Kallweit
2019-04-29 20:14 ` [PATCH net-next 2/2] net: phy: improve phy_set_sym_pause and phy_set_asym_pause Heiner Kallweit
2019-05-01 19:34 ` [PATCH v2 net-next] net: phy: improve pause handling Heiner Kallweit
2 siblings, 0 replies; 9+ messages in thread
From: Heiner Kallweit @ 2019-04-29 20:13 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev
When probing the phy device we set sym and asym pause in the "supported"
bitmap (unless the PHY tells us otherwise). However we don't know yet
whether the MAC supports pause. Simply copying phy->supported to
phy->advertising will trigger advertising pause, and that's not
what we want. Therefore add phy_advertise_supported() that copies all
modes but doesn't touch the pause bits.
In phy_support_(a)sym_pause we shouldn't set any bits in the supported
bitmap because we may set a bit the PHY intentionally disabled.
Effective pause support should be the AND-combined PHY and MAC pause
capabilities. If the MAC supports everything, then it's only relevant
what the PHY supports. If MAC supports sym pause only, then we have to
clear the asym bit in phydev->supported.
Copy the pause flags only and don't touch the modes, because a driver
may have intentionally removed a mode from phydev->advertising.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/fixed_phy.c | 2 +-
drivers/net/phy/phy-core.c | 2 +-
drivers/net/phy/phy_device.c | 36 +++++++++++++++++++++++++++++-------
include/linux/phy.h | 1 +
4 files changed, 32 insertions(+), 9 deletions(-)
diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
index 1acd8bfdb..3ffe46df2 100644
--- a/drivers/net/phy/fixed_phy.c
+++ b/drivers/net/phy/fixed_phy.c
@@ -301,7 +301,7 @@ static struct phy_device *__fixed_phy_register(unsigned int irq,
phy->supported);
}
- linkmode_copy(phy->advertising, phy->supported);
+ phy_advertise_supported(phy);
ret = phy_device_register(phy);
if (ret) {
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 12ce67102..3daf0214a 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -228,7 +228,7 @@ int phy_set_max_speed(struct phy_device *phydev, u32 max_speed)
if (err)
return err;
- linkmode_copy(phydev->advertising, phydev->supported);
+ phy_advertise_supported(phydev);
return 0;
}
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 2a2aaa5f3..544b98b34 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1985,10 +1985,35 @@ EXPORT_SYMBOL(genphy_loopback);
void phy_remove_link_mode(struct phy_device *phydev, u32 link_mode)
{
linkmode_clear_bit(link_mode, phydev->supported);
- linkmode_copy(phydev->advertising, phydev->supported);
+ phy_advertise_supported(phydev);
}
EXPORT_SYMBOL(phy_remove_link_mode);
+static void phy_copy_pause_bits(unsigned long *dst, unsigned long *src)
+{
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, dst,
+ linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, src));
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_Pause_BIT, dst,
+ linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT, src));
+}
+
+/**
+ * phy_advertise_supported - Advertise all supported modes
+ * @phydev: target phy_device struct
+ *
+ * Description: Called to advertise all supported modes, doesn't touch
+ * pause mode advertising.
+ */
+void phy_advertise_supported(struct phy_device *phydev)
+{
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(new);
+
+ linkmode_copy(new, phydev->supported);
+ phy_copy_pause_bits(new, phydev->advertising);
+ linkmode_copy(phydev->advertising, new);
+}
+EXPORT_SYMBOL(phy_advertise_supported);
+
/**
* phy_support_sym_pause - Enable support of symmetrical pause
* @phydev: target phy_device struct
@@ -1999,8 +2024,7 @@ EXPORT_SYMBOL(phy_remove_link_mode);
void phy_support_sym_pause(struct phy_device *phydev)
{
linkmode_clear_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, phydev->supported);
- linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT, phydev->supported);
- linkmode_copy(phydev->advertising, phydev->supported);
+ phy_copy_pause_bits(phydev->advertising, phydev->supported);
}
EXPORT_SYMBOL(phy_support_sym_pause);
@@ -2012,9 +2036,7 @@ EXPORT_SYMBOL(phy_support_sym_pause);
*/
void phy_support_asym_pause(struct phy_device *phydev)
{
- linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT, phydev->supported);
- linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, phydev->supported);
- linkmode_copy(phydev->advertising, phydev->supported);
+ phy_copy_pause_bits(phydev->advertising, phydev->supported);
}
EXPORT_SYMBOL(phy_support_asym_pause);
@@ -2177,7 +2199,7 @@ static int phy_probe(struct device *dev)
phydev->is_gigabit_capable = 1;
of_set_phy_supported(phydev);
- linkmode_copy(phydev->advertising, phydev->supported);
+ phy_advertise_supported(phydev);
/* Get the EEE modes we want to prohibit. We will ask
* the PHY stop advertising these mode later on
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 0f9552b17..4a03f8a46 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1154,6 +1154,7 @@ void phy_request_interrupt(struct phy_device *phydev);
void phy_print_status(struct phy_device *phydev);
int phy_set_max_speed(struct phy_device *phydev, u32 max_speed);
void phy_remove_link_mode(struct phy_device *phydev, u32 link_mode);
+void phy_advertise_supported(struct phy_device *phydev);
void phy_support_sym_pause(struct phy_device *phydev);
void phy_support_asym_pause(struct phy_device *phydev);
void phy_set_sym_pause(struct phy_device *phydev, bool rx, bool tx,
--
2.21.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next 2/2] net: phy: improve phy_set_sym_pause and phy_set_asym_pause
2019-04-29 20:12 [PATCH net-next 0/2] net: phy: improve pause handling Heiner Kallweit
2019-04-29 20:13 ` [PATCH net-next 1/2] " Heiner Kallweit
@ 2019-04-29 20:14 ` Heiner Kallweit
2019-04-29 21:52 ` Andrew Lunn
2019-05-01 19:34 ` [PATCH v2 net-next] net: phy: improve pause handling Heiner Kallweit
2 siblings, 1 reply; 9+ messages in thread
From: Heiner Kallweit @ 2019-04-29 20:14 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev
We should consider what is supported, and we shouldn't mess with
phydev->supported. In addition make sure that phy_set_sym_pause()
clears the asym pause bit in phydev->advertising.
In phy_set_sym_pause() use the same mechanism as in
phy_set_asym_pause() to restart autoneg if needed.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/phy_device.c | 28 ++++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 544b98b34..eb430f2a8 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -2054,13 +2054,24 @@ EXPORT_SYMBOL(phy_support_asym_pause);
void phy_set_sym_pause(struct phy_device *phydev, bool rx, bool tx,
bool autoneg)
{
- linkmode_clear_bit(ETHTOOL_LINK_MODE_Pause_BIT, phydev->supported);
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(oldadv);
+ bool sym_pause_supported;
+
+ sym_pause_supported = linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT,
+ phydev->supported);
+
+ linkmode_copy(oldadv, phydev->advertising);
- if (rx && tx && autoneg)
+ linkmode_clear_bit(ETHTOOL_LINK_MODE_Pause_BIT, phydev->advertising);
+ linkmode_clear_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
+ phydev->advertising);
+
+ if (rx && tx && autoneg && sym_pause_supported)
linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT,
- phydev->supported);
+ phydev->advertising);
- linkmode_copy(phydev->advertising, phydev->supported);
+ if (!linkmode_equal(oldadv, phydev->advertising) && phydev->autoneg)
+ phy_start_aneg(phydev);
}
EXPORT_SYMBOL(phy_set_sym_pause);
@@ -2078,6 +2089,11 @@ EXPORT_SYMBOL(phy_set_sym_pause);
void phy_set_asym_pause(struct phy_device *phydev, bool rx, bool tx)
{
__ETHTOOL_DECLARE_LINK_MODE_MASK(oldadv);
+ bool asym_pause_supported;
+
+ asym_pause_supported =
+ linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
+ phydev->supported);
linkmode_copy(oldadv, phydev->advertising);
@@ -2086,14 +2102,14 @@ void phy_set_asym_pause(struct phy_device *phydev, bool rx, bool tx)
linkmode_clear_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
phydev->advertising);
- if (rx) {
+ if (rx && asym_pause_supported) {
linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT,
phydev->advertising);
linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
phydev->advertising);
}
- if (tx)
+ if (tx && asym_pause_supported)
linkmode_change_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
phydev->advertising);
--
2.21.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 2/2] net: phy: improve phy_set_sym_pause and phy_set_asym_pause
2019-04-29 20:14 ` [PATCH net-next 2/2] net: phy: improve phy_set_sym_pause and phy_set_asym_pause Heiner Kallweit
@ 2019-04-29 21:52 ` Andrew Lunn
2019-04-30 5:06 ` Heiner Kallweit
0 siblings, 1 reply; 9+ messages in thread
From: Andrew Lunn @ 2019-04-29 21:52 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: Florian Fainelli, David Miller, netdev
> @@ -2078,6 +2089,11 @@ EXPORT_SYMBOL(phy_set_sym_pause);
> void phy_set_asym_pause(struct phy_device *phydev, bool rx, bool tx)
> {
> __ETHTOOL_DECLARE_LINK_MODE_MASK(oldadv);
> + bool asym_pause_supported;
> +
> + asym_pause_supported =
> + linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
> + phydev->supported);
>
> linkmode_copy(oldadv, phydev->advertising);
>
> @@ -2086,14 +2102,14 @@ void phy_set_asym_pause(struct phy_device *phydev, bool rx, bool tx)
> linkmode_clear_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
> phydev->advertising);
>
> - if (rx) {
> + if (rx && asym_pause_supported) {
> linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT,
> phydev->advertising);
> linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
> phydev->advertising);
> }
>
> - if (tx)
> + if (tx && asym_pause_supported)
> linkmode_change_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
> phydev->advertising);
Hi Heiner
If the PHY only supports Pause, not Asym Pause, i wounder if we should
fall back to Pause here?
Andrew
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 2/2] net: phy: improve phy_set_sym_pause and phy_set_asym_pause
2019-04-29 21:52 ` Andrew Lunn
@ 2019-04-30 5:06 ` Heiner Kallweit
2019-05-01 19:32 ` Heiner Kallweit
0 siblings, 1 reply; 9+ messages in thread
From: Heiner Kallweit @ 2019-04-30 5:06 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Florian Fainelli, David Miller, netdev
On 29.04.2019 23:52, Andrew Lunn wrote:
>> @@ -2078,6 +2089,11 @@ EXPORT_SYMBOL(phy_set_sym_pause);
>> void phy_set_asym_pause(struct phy_device *phydev, bool rx, bool tx)
>> {
>> __ETHTOOL_DECLARE_LINK_MODE_MASK(oldadv);
>> + bool asym_pause_supported;
>> +
>> + asym_pause_supported =
>> + linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
>> + phydev->supported);
>>
>> linkmode_copy(oldadv, phydev->advertising);
>>
>> @@ -2086,14 +2102,14 @@ void phy_set_asym_pause(struct phy_device *phydev, bool rx, bool tx)
>> linkmode_clear_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
>> phydev->advertising);
>>
>> - if (rx) {
>> + if (rx && asym_pause_supported) {
>> linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT,
>> phydev->advertising);
>> linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
>> phydev->advertising);
>> }
>>
>> - if (tx)
>> + if (tx && asym_pause_supported)
>> linkmode_change_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
>> phydev->advertising);
>
> Hi Heiner
>
Hi Andrew,
> If the PHY only supports Pause, not Asym Pause, i wounder if we should
> fall back to Pause here?
>
I wasn't sure about whether a silent fallback is the expected behavior.
Also open is whether we can rely on a set_pause callback having called
phy_validate_pause() before. Another option could be to change the
return type to int and return an error like -EOPNOTSUPP if the requested
mode isn't supported.
> Andrew
>
Heiner
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 2/2] net: phy: improve phy_set_sym_pause and phy_set_asym_pause
2019-04-30 5:06 ` Heiner Kallweit
@ 2019-05-01 19:32 ` Heiner Kallweit
0 siblings, 0 replies; 9+ messages in thread
From: Heiner Kallweit @ 2019-05-01 19:32 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Florian Fainelli, David Miller, netdev
On 30.04.2019 07:06, Heiner Kallweit wrote:
> On 29.04.2019 23:52, Andrew Lunn wrote:
>>> @@ -2078,6 +2089,11 @@ EXPORT_SYMBOL(phy_set_sym_pause);
>>> void phy_set_asym_pause(struct phy_device *phydev, bool rx, bool tx)
>>> {
>>> __ETHTOOL_DECLARE_LINK_MODE_MASK(oldadv);
>>> + bool asym_pause_supported;
>>> +
>>> + asym_pause_supported =
>>> + linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
>>> + phydev->supported);
>>>
>>> linkmode_copy(oldadv, phydev->advertising);
>>>
>>> @@ -2086,14 +2102,14 @@ void phy_set_asym_pause(struct phy_device *phydev, bool rx, bool tx)
>>> linkmode_clear_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
>>> phydev->advertising);
>>>
>>> - if (rx) {
>>> + if (rx && asym_pause_supported) {
>>> linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT,
>>> phydev->advertising);
>>> linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
>>> phydev->advertising);
>>> }
>>>
>>> - if (tx)
>>> + if (tx && asym_pause_supported)
>>> linkmode_change_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
>>> phydev->advertising);
>>
>> Hi Heiner
>>
> Hi Andrew,
>
>> If the PHY only supports Pause, not Asym Pause, i wounder if we should
>> fall back to Pause here?
>>
> I wasn't sure about whether a silent fallback is the expected behavior.
> Also open is whether we can rely on a set_pause callback having called
> phy_validate_pause() before. Another option could be to change the
> return type to int and return an error like -EOPNOTSUPP if the requested
> mode isn't supported.
>
Most drivers call phy_validate_pause() first, this seems to be the
expected pattern. Therefore I will remove patch 2. However there seems to
be an error in phy_validate_pause(), the fix I will submit separately.
>> Andrew
>>
> Heiner
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 net-next] net: phy: improve pause handling
2019-04-29 20:12 [PATCH net-next 0/2] net: phy: improve pause handling Heiner Kallweit
2019-04-29 20:13 ` [PATCH net-next 1/2] " Heiner Kallweit
2019-04-29 20:14 ` [PATCH net-next 2/2] net: phy: improve phy_set_sym_pause and phy_set_asym_pause Heiner Kallweit
@ 2019-05-01 19:34 ` Heiner Kallweit
2019-05-01 20:35 ` Andrew Lunn
2019-05-04 4:48 ` David Miller
2 siblings, 2 replies; 9+ messages in thread
From: Heiner Kallweit @ 2019-05-01 19:34 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev
When probing the phy device we set sym and asym pause in the "supported"
bitmap (unless the PHY tells us otherwise). However we don't know yet
whether the MAC supports pause. Simply copying phy->supported to
phy->advertising will trigger advertising pause, and that's not
what we want. Therefore add phy_advertise_supported() that copies all
modes but doesn't touch the pause bits.
In phy_support_(a)sym_pause we shouldn't set any bits in the supported
bitmap because we may set a bit the PHY intentionally disabled.
Effective pause support should be the AND-combined PHY and MAC pause
capabilities. If the MAC supports everything, then it's only relevant
what the PHY supports. If MAC supports sym pause only, then we have to
clear the asym bit in phydev->supported.
Copy the pause flags only and don't touch the modes, because a driver
may have intentionally removed a mode from phydev->advertising.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- removed patch 2 from the series
---
drivers/net/phy/fixed_phy.c | 2 +-
drivers/net/phy/phy-core.c | 2 +-
drivers/net/phy/phy_device.c | 36 +++++++++++++++++++++++++++++-------
include/linux/phy.h | 1 +
4 files changed, 32 insertions(+), 9 deletions(-)
diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
index 1acd8bfdb..3ffe46df2 100644
--- a/drivers/net/phy/fixed_phy.c
+++ b/drivers/net/phy/fixed_phy.c
@@ -301,7 +301,7 @@ static struct phy_device *__fixed_phy_register(unsigned int irq,
phy->supported);
}
- linkmode_copy(phy->advertising, phy->supported);
+ phy_advertise_supported(phy);
ret = phy_device_register(phy);
if (ret) {
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 12ce67102..3daf0214a 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -228,7 +228,7 @@ int phy_set_max_speed(struct phy_device *phydev, u32 max_speed)
if (err)
return err;
- linkmode_copy(phydev->advertising, phydev->supported);
+ phy_advertise_supported(phydev);
return 0;
}
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 2a2aaa5f3..544b98b34 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1985,10 +1985,35 @@ EXPORT_SYMBOL(genphy_loopback);
void phy_remove_link_mode(struct phy_device *phydev, u32 link_mode)
{
linkmode_clear_bit(link_mode, phydev->supported);
- linkmode_copy(phydev->advertising, phydev->supported);
+ phy_advertise_supported(phydev);
}
EXPORT_SYMBOL(phy_remove_link_mode);
+static void phy_copy_pause_bits(unsigned long *dst, unsigned long *src)
+{
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, dst,
+ linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, src));
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_Pause_BIT, dst,
+ linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT, src));
+}
+
+/**
+ * phy_advertise_supported - Advertise all supported modes
+ * @phydev: target phy_device struct
+ *
+ * Description: Called to advertise all supported modes, doesn't touch
+ * pause mode advertising.
+ */
+void phy_advertise_supported(struct phy_device *phydev)
+{
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(new);
+
+ linkmode_copy(new, phydev->supported);
+ phy_copy_pause_bits(new, phydev->advertising);
+ linkmode_copy(phydev->advertising, new);
+}
+EXPORT_SYMBOL(phy_advertise_supported);
+
/**
* phy_support_sym_pause - Enable support of symmetrical pause
* @phydev: target phy_device struct
@@ -1999,8 +2024,7 @@ EXPORT_SYMBOL(phy_remove_link_mode);
void phy_support_sym_pause(struct phy_device *phydev)
{
linkmode_clear_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, phydev->supported);
- linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT, phydev->supported);
- linkmode_copy(phydev->advertising, phydev->supported);
+ phy_copy_pause_bits(phydev->advertising, phydev->supported);
}
EXPORT_SYMBOL(phy_support_sym_pause);
@@ -2012,9 +2036,7 @@ EXPORT_SYMBOL(phy_support_sym_pause);
*/
void phy_support_asym_pause(struct phy_device *phydev)
{
- linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT, phydev->supported);
- linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, phydev->supported);
- linkmode_copy(phydev->advertising, phydev->supported);
+ phy_copy_pause_bits(phydev->advertising, phydev->supported);
}
EXPORT_SYMBOL(phy_support_asym_pause);
@@ -2177,7 +2199,7 @@ static int phy_probe(struct device *dev)
phydev->is_gigabit_capable = 1;
of_set_phy_supported(phydev);
- linkmode_copy(phydev->advertising, phydev->supported);
+ phy_advertise_supported(phydev);
/* Get the EEE modes we want to prohibit. We will ask
* the PHY stop advertising these mode later on
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 0f9552b17..4a03f8a46 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1154,6 +1154,7 @@ void phy_request_interrupt(struct phy_device *phydev);
void phy_print_status(struct phy_device *phydev);
int phy_set_max_speed(struct phy_device *phydev, u32 max_speed);
void phy_remove_link_mode(struct phy_device *phydev, u32 link_mode);
+void phy_advertise_supported(struct phy_device *phydev);
void phy_support_sym_pause(struct phy_device *phydev);
void phy_support_asym_pause(struct phy_device *phydev);
void phy_set_sym_pause(struct phy_device *phydev, bool rx, bool tx,
--
2.21.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 net-next] net: phy: improve pause handling
2019-05-01 19:34 ` [PATCH v2 net-next] net: phy: improve pause handling Heiner Kallweit
@ 2019-05-01 20:35 ` Andrew Lunn
2019-05-04 4:48 ` David Miller
1 sibling, 0 replies; 9+ messages in thread
From: Andrew Lunn @ 2019-05-01 20:35 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: Florian Fainelli, David Miller, netdev
On Wed, May 01, 2019 at 09:34:43PM +0200, Heiner Kallweit wrote:
> When probing the phy device we set sym and asym pause in the "supported"
> bitmap (unless the PHY tells us otherwise). However we don't know yet
> whether the MAC supports pause. Simply copying phy->supported to
> phy->advertising will trigger advertising pause, and that's not
> what we want. Therefore add phy_advertise_supported() that copies all
> modes but doesn't touch the pause bits.
>
> In phy_support_(a)sym_pause we shouldn't set any bits in the supported
> bitmap because we may set a bit the PHY intentionally disabled.
> Effective pause support should be the AND-combined PHY and MAC pause
> capabilities. If the MAC supports everything, then it's only relevant
> what the PHY supports. If MAC supports sym pause only, then we have to
> clear the asym bit in phydev->supported.
> Copy the pause flags only and don't touch the modes, because a driver
> may have intentionally removed a mode from phydev->advertising.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 net-next] net: phy: improve pause handling
2019-05-01 19:34 ` [PATCH v2 net-next] net: phy: improve pause handling Heiner Kallweit
2019-05-01 20:35 ` Andrew Lunn
@ 2019-05-04 4:48 ` David Miller
1 sibling, 0 replies; 9+ messages in thread
From: David Miller @ 2019-05-04 4:48 UTC (permalink / raw)
To: hkallweit1; +Cc: andrew, f.fainelli, netdev
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Wed, 1 May 2019 21:34:43 +0200
> When probing the phy device we set sym and asym pause in the "supported"
> bitmap (unless the PHY tells us otherwise). However we don't know yet
> whether the MAC supports pause. Simply copying phy->supported to
> phy->advertising will trigger advertising pause, and that's not
> what we want. Therefore add phy_advertise_supported() that copies all
> modes but doesn't touch the pause bits.
>
> In phy_support_(a)sym_pause we shouldn't set any bits in the supported
> bitmap because we may set a bit the PHY intentionally disabled.
> Effective pause support should be the AND-combined PHY and MAC pause
> capabilities. If the MAC supports everything, then it's only relevant
> what the PHY supports. If MAC supports sym pause only, then we have to
> clear the asym bit in phydev->supported.
> Copy the pause flags only and don't touch the modes, because a driver
> may have intentionally removed a mode from phydev->advertising.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> v2:
> - removed patch 2 from the series
Applied.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-05-04 4:48 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-29 20:12 [PATCH net-next 0/2] net: phy: improve pause handling Heiner Kallweit
2019-04-29 20:13 ` [PATCH net-next 1/2] " Heiner Kallweit
2019-04-29 20:14 ` [PATCH net-next 2/2] net: phy: improve phy_set_sym_pause and phy_set_asym_pause Heiner Kallweit
2019-04-29 21:52 ` Andrew Lunn
2019-04-30 5:06 ` Heiner Kallweit
2019-05-01 19:32 ` Heiner Kallweit
2019-05-01 19:34 ` [PATCH v2 net-next] net: phy: improve pause handling Heiner Kallweit
2019-05-01 20:35 ` Andrew Lunn
2019-05-04 4:48 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.