Subject: Re: [RFC net-next 4/4] net: phy: Correctly process PHY_HALTED in phy_stop_machine()

From: Florian Fainelli <[email-masked]>
Date: Tue, 31 Oct 2017 09:33:14 -0700

On 10/31/2017 08:26 AM, Geert Uytterhoeven wrote:
> Hi Florian,
> 
> On Mon, Oct 30, 2017 at 5:09 PM, Florian Fainelli <[email-masked]> wrote:
>> On 10/30/2017 06:56 AM, Geert Uytterhoeven wrote:
>>> On Thu, Oct 26, 2017 at 1:21 AM, Florian Fainelli <[email-masked]> wrote:
>>>> Marc reported that he was not getting the PHY library adjust_link()
>>>> callback function to run when calling phy_stop() + phy_disconnect()
>>>> which does not indeed happen because we set the state machine to
>>>> PHY_HALTED but we don't get to run it to process this state past that
>>>> point.
>>>>
>>>> Fix this with a synchronous call to phy_state_machine() in order to have
>>>> the state machine actually act on PHY_HALTED, set the PHY device's link
>>>> down, turn the network device's carrier off and finally call the
>>>> adjust_link() function.
>>>>
>>>> At the end of phy_state_machine() though, if we are going to be moving
>>>> from PHY_HALTED to PHY_HALTED, do not reschedule the state machine, this
>>>> is pointless.
>>>>
>>>> Reported-by: Marc Gonzalez <[email-masked]>
>>>> Fixes: a390d1f379cf ("phylib: convert state_queue work to delayed_work")
>>>> Signed-off-by: Marc Gonzalez <[email-masked]>
>>>> Signed-off-by: Florian Fainelli <[email-masked]>
>>>
>>> Thanks for your patch!
>>>
>>> Unfortunately, after applying this one, the last in your series, both
>>> sh73a0/kzm9g and r8a73a4/ape6evm start crashing again in the system
>>> suspend/resume path, due to register accesses while the device is already
>>> suspended:
>>
>> OK, seems like there is another path, uncovered by this patch that we
>> can be hitting, does the following patch below help?
> 
> Unfortunately it doesn't help.

OK :/

> 
>>> Unhandled fault: imprecise external abort (0x1406) at 0x0005b950
> 
> Note that this is an imprecise external abort, i.e. it's reporting may
> be delayed,
> and the backtrace may be inaccurate.

True, can you help narrow it down with me? Can you confirm that
adjust_link() (assuming that is the problem) does not get called past
phy_stop_machine() as it should?
-- 
Florian