Hi Russell,
On 11/30/2017 07:28 AM, Russell King - ARM Linux wrote:
> On Thu, Nov 30, 2017 at 10:10:18AM +0000, Russell King - ARM Linux wrote:
>> On Thu, Nov 30, 2017 at 08:51:21AM +0000, Yan Markman wrote:
>>> The phylink_stop is called before phylink_disconnect_phy
>>> You could see in mvpp2.c:
>>>
>>> mvpp2_stop_dev() {
>>> phylink_stop(port->phylink);
>>> }
>>>
>>> mvpp2_stop() {
>>> mvpp2_stop_dev(port);
>>> phylink_disconnect_phy(port->phylink);
>>> }
>>>
>>> .ndo_stop = mvpp2_stop,
>>
>> Sorry, I don't have this in mvpp2.c, so I have no visibility of what
>> you're working with.
>>
>> What you have above looks correct, and I see no reason why the p21
>> patch would not have resolved your issue. The p21 patch ensures
>> that phylink_resolve() gets called and completes before phylink_stop()
>> returns. In that case, phylink_resolve() will call the mac_link_down()
>> method if the link is not already down. It will also print the "Link
>> is Down" message.
>>
>> Florian has already tested this patch after encountering a similar
>> issue, and has reported that it solves the problem for him. I've also
>> tested it with mvneta, and the original mvpp2x driver on Macchiatobin.
>>
>> Maybe there's something different about mvpp2, but as I have no
>> visibility of that driver and the modifications therein, I can't
>> comment further other than stating that it works for three different
>> implementations.
>>
>> Maybe you could try and work out what's going on with the p21 patch
>> in your case?
>
> I think I now realise what's probably going on.
>
> If you call netif_carrier_off() before phylink_stop(), then phylink will
> believe that the link is already down, and so it won't bother calling
> mac_link_down() - it will believe that the link is already down.
>
> I'll update the documentation for phylink_stop() to spell out this
> aspect.
>
There are pretty high number of net drivers which do call
netif_carrier_off(dev);
before
phy_stop(dev->phydev);
in .ndo_stop() callback.
As per you comment this seems to be incorrect, so should such calls be removed?
--
regards,
-grygorii
