* [RFC PATCH net-next] net: phy: force phy suspend when calling phy_stop
@ 2019-08-27 2:47 Jian Shen
2019-08-27 5:51 ` Heiner Kallweit
2019-08-27 10:11 ` Sergei Shtylyov
0 siblings, 2 replies; 7+ messages in thread
From: Jian Shen @ 2019-08-27 2:47 UTC (permalink / raw)
To: andrew, f.fainelli, hkallweit1, davem; +Cc: netdev, forest.zhouchang, linuxarm
Some ethernet drivers may call phy_start() and phy_stop() from
ndo_open and ndo_close() respectively.
When network cable is unconnected, and operate like below:
step 1: ifconfig ethX up -> ndo_open -> phy_start ->start
autoneg, and phy is no link.
step 2: ifconfig ethX down -> ndo_close -> phy_stop -> just stop
phy state machine.
step 3: plugin the network cable, and autoneg complete, then
LED for link status will be on.
step 4: ethtool ethX --> see the result of "Link detected" is no.
This patch forces phy suspend even phydev->link is off.
Signed-off-by: Jian Shen <shenjian15@huawei.com>
---
drivers/net/phy/phy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index f3adea9..0acd5b4 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -911,8 +911,8 @@ void phy_state_machine(struct work_struct *work)
if (phydev->link) {
phydev->link = 0;
phy_link_down(phydev, true);
- do_suspend = true;
}
+ do_suspend = true;
break;
}
--
2.8.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC PATCH net-next] net: phy: force phy suspend when calling phy_stop
2019-08-27 2:47 [RFC PATCH net-next] net: phy: force phy suspend when calling phy_stop Jian Shen
@ 2019-08-27 5:51 ` Heiner Kallweit
2019-08-27 8:29 ` shenjian (K)
2019-08-27 10:11 ` Sergei Shtylyov
1 sibling, 1 reply; 7+ messages in thread
From: Heiner Kallweit @ 2019-08-27 5:51 UTC (permalink / raw)
To: Jian Shen, andrew, f.fainelli, davem; +Cc: netdev, forest.zhouchang, linuxarm
On 27.08.2019 04:47, Jian Shen wrote:
> Some ethernet drivers may call phy_start() and phy_stop() from
> ndo_open and ndo_close() respectively.
>
> When network cable is unconnected, and operate like below:
> step 1: ifconfig ethX up -> ndo_open -> phy_start ->start
> autoneg, and phy is no link.
> step 2: ifconfig ethX down -> ndo_close -> phy_stop -> just stop
> phy state machine.
> step 3: plugin the network cable, and autoneg complete, then
> LED for link status will be on.
> step 4: ethtool ethX --> see the result of "Link detected" is no.
>
Step 3 and 4 seem to be unrelated to the actual issue.
With which MAC + PHY driver did you observe this?
> This patch forces phy suspend even phydev->link is off.
>
> Signed-off-by: Jian Shen <shenjian15@huawei.com>
> ---
> drivers/net/phy/phy.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index f3adea9..0acd5b4 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -911,8 +911,8 @@ void phy_state_machine(struct work_struct *work)
> if (phydev->link) {
> phydev->link = 0;
> phy_link_down(phydev, true);
> - do_suspend = true;
> }
> + do_suspend = true;
> break;
> }
>
>
Heiner
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH net-next] net: phy: force phy suspend when calling phy_stop
2019-08-27 5:51 ` Heiner Kallweit
@ 2019-08-27 8:29 ` shenjian (K)
2019-08-27 19:41 ` Heiner Kallweit
0 siblings, 1 reply; 7+ messages in thread
From: shenjian (K) @ 2019-08-27 8:29 UTC (permalink / raw)
To: Heiner Kallweit, andrew, f.fainelli, davem
Cc: netdev, forest.zhouchang, linuxarm
在 2019/8/27 13:51, Heiner Kallweit 写道:
> On 27.08.2019 04:47, Jian Shen wrote:
>> Some ethernet drivers may call phy_start() and phy_stop() from
>> ndo_open and ndo_close() respectively.
>>
>> When network cable is unconnected, and operate like below:
>> step 1: ifconfig ethX up -> ndo_open -> phy_start ->start
>> autoneg, and phy is no link.
>> step 2: ifconfig ethX down -> ndo_close -> phy_stop -> just stop
>> phy state machine.
>> step 3: plugin the network cable, and autoneg complete, then
>> LED for link status will be on.
>> step 4: ethtool ethX --> see the result of "Link detected" is no.
>>
> Step 3 and 4 seem to be unrelated to the actual issue.
> With which MAC + PHY driver did you observe this?
>
Thanks Heiner,
I tested this on HNS3 driver, with two phy, Marvell 88E1512 and RTL8211.
Step 3 and Step 4 is just to describe that the LED of link shows link up,
but the port information shows no link.
>> This patch forces phy suspend even phydev->link is off.
>>
>> Signed-off-by: Jian Shen <shenjian15@huawei.com>
>> ---
>> drivers/net/phy/phy.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>> index f3adea9..0acd5b4 100644
>> --- a/drivers/net/phy/phy.c
>> +++ b/drivers/net/phy/phy.c
>> @@ -911,8 +911,8 @@ void phy_state_machine(struct work_struct *work)
>> if (phydev->link) {
>> phydev->link = 0;
>> phy_link_down(phydev, true);
>> - do_suspend = true;
>> }
>> + do_suspend = true;
>> break;
>> }
>>
>>
> Heiner
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH net-next] net: phy: force phy suspend when calling phy_stop
2019-08-27 2:47 [RFC PATCH net-next] net: phy: force phy suspend when calling phy_stop Jian Shen
2019-08-27 5:51 ` Heiner Kallweit
@ 2019-08-27 10:11 ` Sergei Shtylyov
2019-08-27 11:36 ` shenjian (K)
1 sibling, 1 reply; 7+ messages in thread
From: Sergei Shtylyov @ 2019-08-27 10:11 UTC (permalink / raw)
To: Jian Shen, andrew, f.fainelli, hkallweit1, davem
Cc: netdev, forest.zhouchang, linuxarm
On 27.08.2019 5:47, Jian Shen wrote:
> Some ethernet drivers may call phy_start() and phy_stop() from
> ndo_open and ndo_close() respectively.
ndo_open() for consistency.
> When network cable is unconnected, and operate like below:
> step 1: ifconfig ethX up -> ndo_open -> phy_start ->start
> autoneg, and phy is no link.
> step 2: ifconfig ethX down -> ndo_close -> phy_stop -> just stop
> phy state machine.
> step 3: plugin the network cable, and autoneg complete, then
> LED for link status will be on.
> step 4: ethtool ethX --> see the result of "Link detected" is no.
>
> This patch forces phy suspend even phydev->link is off.
>
> Signed-off-by: Jian Shen <shenjian15@huawei.com>
[...]
MBR, Sergei
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH net-next] net: phy: force phy suspend when calling phy_stop
2019-08-27 10:11 ` Sergei Shtylyov
@ 2019-08-27 11:36 ` shenjian (K)
0 siblings, 0 replies; 7+ messages in thread
From: shenjian (K) @ 2019-08-27 11:36 UTC (permalink / raw)
To: Sergei Shtylyov, andrew, f.fainelli, hkallweit1, davem
Cc: netdev, forest.zhouchang, linuxarm
在 2019/8/27 18:11, Sergei Shtylyov 写道:
> On 27.08.2019 5:47, Jian Shen wrote:
>
>> Some ethernet drivers may call phy_start() and phy_stop() from
>> ndo_open and ndo_close() respectively.
>
> ndo_open() for consistency.
>
>> When network cable is unconnected, and operate like below:
>> step 1: ifconfig ethX up -> ndo_open -> phy_start ->start
>> autoneg, and phy is no link.
>> step 2: ifconfig ethX down -> ndo_close -> phy_stop -> just stop
>> phy state machine.
>> step 3: plugin the network cable, and autoneg complete, then
>> LED for link status will be on.
>> step 4: ethtool ethX --> see the result of "Link detected" is no.
>>
>> This patch forces phy suspend even phydev->link is off.
>>
>> Signed-off-by: Jian Shen <shenjian15@huawei.com>
> [...]
>
> MBR, Sergei
>
>
Thanks, will fix it.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH net-next] net: phy: force phy suspend when calling phy_stop
2019-08-27 8:29 ` shenjian (K)
@ 2019-08-27 19:41 ` Heiner Kallweit
2019-08-28 1:03 ` shenjian (K)
0 siblings, 1 reply; 7+ messages in thread
From: Heiner Kallweit @ 2019-08-27 19:41 UTC (permalink / raw)
To: shenjian (K), andrew, f.fainelli, davem
Cc: netdev, forest.zhouchang, linuxarm
On 27.08.2019 10:29, shenjian (K) wrote:
>
>
> 在 2019/8/27 13:51, Heiner Kallweit 写道:
>> On 27.08.2019 04:47, Jian Shen wrote:
>>> Some ethernet drivers may call phy_start() and phy_stop() from
>>> ndo_open and ndo_close() respectively.
>>>
>>> When network cable is unconnected, and operate like below:
>>> step 1: ifconfig ethX up -> ndo_open -> phy_start ->start
>>> autoneg, and phy is no link.
>>> step 2: ifconfig ethX down -> ndo_close -> phy_stop -> just stop
>>> phy state machine.
>>> step 3: plugin the network cable, and autoneg complete, then
>>> LED for link status will be on.
>>> step 4: ethtool ethX --> see the result of "Link detected" is no.
>>>
>> Step 3 and 4 seem to be unrelated to the actual issue.
>> With which MAC + PHY driver did you observe this?
>>
> Thanks Heiner,
>
> I tested this on HNS3 driver, with two phy, Marvell 88E1512 and RTL8211.
>
> Step 3 and Step 4 is just to describe that the LED of link shows link up,
> but the port information shows no link.
>
ethtool refers to the link at MAC level. Therefore default implementation
ethtool_op_get_link just returns the result of netif_carrier_ok().
Also using PHY link status if interface is down doesn't really make sense:
- phylib state machine isn't running, therefore PHY status doesn't get updated
- often MAC drivers shut down parts of the MAC on ndo_close, this typically
makes the internal MDIO bus unaccessible
So just remove steps 3 and 4. The patch itself is fine with me.
>
>>> This patch forces phy suspend even phydev->link is off.
>>>
>>> Signed-off-by: Jian Shen <shenjian15@huawei.com>
>>> ---
>>> drivers/net/phy/phy.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>>> index f3adea9..0acd5b4 100644
>>> --- a/drivers/net/phy/phy.c
>>> +++ b/drivers/net/phy/phy.c
>>> @@ -911,8 +911,8 @@ void phy_state_machine(struct work_struct *work)
>>> if (phydev->link) {
>>> phydev->link = 0;
>>> phy_link_down(phydev, true);
>>> - do_suspend = true;
>>> }
>>> + do_suspend = true;
>>> break;
>>> }
>>>
>>>
>> Heiner
>>
>>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH net-next] net: phy: force phy suspend when calling phy_stop
2019-08-27 19:41 ` Heiner Kallweit
@ 2019-08-28 1:03 ` shenjian (K)
0 siblings, 0 replies; 7+ messages in thread
From: shenjian (K) @ 2019-08-28 1:03 UTC (permalink / raw)
To: Heiner Kallweit, andrew, f.fainelli, davem
Cc: netdev, forest.zhouchang, linuxarm
在 2019/8/28 3:41, Heiner Kallweit 写道:
> On 27.08.2019 10:29, shenjian (K) wrote:
>>
>>
>> 在 2019/8/27 13:51, Heiner Kallweit 写道:
>>> On 27.08.2019 04:47, Jian Shen wrote:
>>>> Some ethernet drivers may call phy_start() and phy_stop() from
>>>> ndo_open and ndo_close() respectively.
>>>>
>>>> When network cable is unconnected, and operate like below:
>>>> step 1: ifconfig ethX up -> ndo_open -> phy_start ->start
>>>> autoneg, and phy is no link.
>>>> step 2: ifconfig ethX down -> ndo_close -> phy_stop -> just stop
>>>> phy state machine.
>>>> step 3: plugin the network cable, and autoneg complete, then
>>>> LED for link status will be on.
>>>> step 4: ethtool ethX --> see the result of "Link detected" is no.
>>>>
>>> Step 3 and 4 seem to be unrelated to the actual issue.
>>> With which MAC + PHY driver did you observe this?
>>>
>> Thanks Heiner,
>>
>> I tested this on HNS3 driver, with two phy, Marvell 88E1512 and RTL8211.
>>
>> Step 3 and Step 4 is just to describe that the LED of link shows link up,
>> but the port information shows no link.
>>
> ethtool refers to the link at MAC level. Therefore default implementation
> ethtool_op_get_link just returns the result of netif_carrier_ok().
> Also using PHY link status if interface is down doesn't really make sense:
> - phylib state machine isn't running, therefore PHY status doesn't get updated
> - often MAC drivers shut down parts of the MAC on ndo_close, this typically
> makes the internal MDIO bus unaccessible
> So just remove steps 3 and 4. The patch itself is fine with me.
>
OK, I will fix the commit log.
Thanks, Heiner.
>>
>>>> This patch forces phy suspend even phydev->link is off.
>>>>
>>>> Signed-off-by: Jian Shen <shenjian15@huawei.com>
>>>> ---
>>>> drivers/net/phy/phy.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>>>> index f3adea9..0acd5b4 100644
>>>> --- a/drivers/net/phy/phy.c
>>>> +++ b/drivers/net/phy/phy.c
>>>> @@ -911,8 +911,8 @@ void phy_state_machine(struct work_struct *work)
>>>> if (phydev->link) {
>>>> phydev->link = 0;
>>>> phy_link_down(phydev, true);
>>>> - do_suspend = true;
>>>> }
>>>> + do_suspend = true;
>>>> break;
>>>> }
>>>>
>>>>
>>> Heiner
>>>
>>>
>>
>>
>
>
> .
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-08-28 1:03 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-27 2:47 [RFC PATCH net-next] net: phy: force phy suspend when calling phy_stop Jian Shen
2019-08-27 5:51 ` Heiner Kallweit
2019-08-27 8:29 ` shenjian (K)
2019-08-27 19:41 ` Heiner Kallweit
2019-08-28 1:03 ` shenjian (K)
2019-08-27 10:11 ` Sergei Shtylyov
2019-08-27 11:36 ` shenjian (K)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).