Re: [PATCH v2] net: stmmac: bug fix to synchronize stmmac_open and stmmac_dvr_probe

[Florian Fainelli]

On 12/28/2016 04:28 PM, Kweh, Hock Leong wrote:
>> Although this is required, we can't be doing it in all circumstances, we 
>> need to
>> mimic what stmmac_drv_remove() does.
>>
>> Let me submit an incremental fix which takes care of mdio bus unregistration.
>> --
>> Florian
> 
> Noted & Thanks. Will test it out once you submitted.

It's done:

https://www.spinics.net/lists/netdev/msg411934.html
-- 
Florian

RE: [PATCH v2] net: stmmac: bug fix to synchronize stmmac_open and stmmac_dvr_probe

[Kweh, Hock Leong]

> -Original Message-
> From: Florian Fainelli [mailto:f.faine...@gmail.com]
> Sent: Thursday, December 29, 2016 2:43 AM
> To: Kweh, Hock Leong <hock.leong.k...@intel.com>; David Miller
> <da...@davemloft.net>
> Cc: joao.pi...@synopsys.com; peppe.cavall...@st.com;
> seraphin.bonna...@st.com; alexandre.tor...@gmail.com;
> manab...@gmail.com; niklas.cas...@axis.com; jo...@kernel.org;
> pa...@ucw.cz; Ong, Boon Leong <boon.leong@intel.com>; Voon, Weifeng
> <weifeng.v...@intel.com>; lars.pers...@axis.com; netdev@vger.kernel.org;
> linux-ker...@vger.kernel.org
> Subject: Re: [PATCH v2] net: stmmac: bug fix to synchronize stmmac_open and
> stmmac_dvr_probe
> 
> On 12/27/2016 09:49 PM, Kweh, Hock Leong wrote:
> >> -Original Message-
> >> From: David Miller [mailto:da...@davemloft.net]
> >> Sent: Wednesday, December 28, 2016 12:34 AM
> >> To: Kweh, Hock Leong <hock.leong.k...@intel.com>
> >> Cc: joao.pi...@synopsys.com; peppe.cavall...@st.com;
> >> seraphin.bonna...@st.com; f.faine...@gmail.com;
> >> alexandre.tor...@gmail.com; manab...@gmail.com;
> >> niklas.cas...@axis.com; jo...@kernel.org; pa...@ucw.cz; Ong, Boon
> >> Leong <boon.leong....@intel.com>; Voon, Weifeng
> >> <weifeng.v...@intel.com>; lars.pers...@axis.com;
> >> netdev@vger.kernel.org; linux-ker...@vger.kernel.org
> >> Subject: Re: [PATCH v2] net: stmmac: bug fix to synchronize
> >> stmmac_open and stmmac_dvr_probe
> >>
> >> From: "Kweh, Hock Leong"   <hock.leong.k...@intel.com>
> >> Date: Tue, 27 Dec 2016 22:42:36 +0800
> >>
> >>> From: "Kweh, Hock Leong" <hock.leong.k...@intel.com>
> >>
> >> You are not the author of this change, do not take credit for it.
> >>
> >> You have copied Florian's patch character by character, therefore he
> >> is the author.
> >>
> >> You also didn't CC: the netdev mailing list properly.
> >
> > Hi David & Florian,
> >
> > Just to clarify that I do not copy exactly from Florian.
> > I have changed it to have proper handling on mdio unregister while
> > netdev_register() failed as showed below:
> >
> > return 0;
> >
> > -error_mdio_register:
> > -   unregister_netdev(ndev);
> >  error_netdev_register:
> > +   stmmac_mdio_unregister(ndev);
> 
> Although this is required, we can't be doing it in all circumstances, we need 
> to
> mimic what stmmac_drv_remove() does.
> 
> Let me submit an incremental fix which takes care of mdio bus unregistration.
> --
> Florian

Noted & Thanks. Will test it out once you submitted.

Thanks & Regards,
Wilson

RE: [PATCH v2] net: stmmac: bug fix to synchronize stmmac_open and stmmac_dvr_probe

[Kweh, Hock Leong]

> -Original Message-
> From: Kishan Sandeep [mailto:sandeepkishan...@gmail.com]
> Sent: Wednesday, December 28, 2016 7:56 PM
> To: Kweh, Hock Leong <hock.leong.k...@intel.com>
> Cc: David Miller <da...@davemloft.net>; f.faine...@gmail.com;
> joao.pi...@synopsys.com; peppe.cavall...@st.com;
> seraphin.bonna...@st.com; alexandre.tor...@gmail.com;
> manab...@gmail.com; niklas.cas...@axis.com; jo...@kernel.org;
> pa...@ucw.cz; lars.pers...@axis.com; netdev@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Subject: Re: [PATCH v2] net: stmmac: bug fix to synchronize stmmac_open and
> stmmac_dvr_probe
> 
> On Wed, Dec 28, 2016 at 7:10 AM, Kweh, Hock Leong
> <hock.leong.k...@intel.com> wrote:
> >> -Original Message-
> >> From: David Miller [mailto:da...@davemloft.net]
> >> Sent: Wednesday, December 28, 2016 12:34 AM
> >> To: Kweh, Hock Leong <hock.leong.k...@intel.com>
> >> Cc: joao.pi...@synopsys.com; peppe.cavall...@st.com;
> >> seraphin.bonna...@st.com; f.faine...@gmail.com;
> >> alexandre.tor...@gmail.com; manab...@gmail.com;
> >> niklas.cas...@axis.com; jo...@kernel.org; pa...@ucw.cz; Ong, Boon
> >> Leong <boon.leong@intel.com>; Voon, Weifeng
> >> <weifeng.v...@intel.com>; lars.pers...@axis.com;
> >> netdev@vger.kernel.org; linux-ker...@vger.kernel.org
> >> Subject: Re: [PATCH v2] net: stmmac: bug fix to synchronize
> >> stmmac_open and stmmac_dvr_probe
> >>
> >> From: "Kweh, Hock Leong"  <hock.leong.k...@intel.com>
> >> Date: Tue, 27 Dec 2016 22:42:36 +0800
> >>
> >> > From: "Kweh, Hock Leong" <hock.leong.k...@intel.com>
> >>
> >> You are not the author of this change, do not take credit for it.
> >>
> >> You have copied Florian's patch character by character, therefore he
> >> is the author.
> >>
> >> You also didn't CC: the netdev mailing list properly.
> >
> > Noted & Thanks.
> >
> > Hi Florian, could you submit this fix from your side so that you are the 
> > author.
> > I will help to test out.
> >
> > Thanks & Regards,
> > Wilson
> >
> I think you can give *--author* for giving author name. Try git commit -am
> "commit message" --author="Author_name "

Oh ... I am not aware of that. Thanks for informing. 
:-)

Regards,
Wilson

Re: [PATCH v2] net: stmmac: bug fix to synchronize stmmac_open and stmmac_dvr_probe

[Florian Fainelli]

On 12/27/2016 09:49 PM, Kweh, Hock Leong wrote:
>> -Original Message-
>> From: David Miller [mailto:da...@davemloft.net]
>> Sent: Wednesday, December 28, 2016 12:34 AM
>> To: Kweh, Hock Leong <hock.leong.k...@intel.com>
>> Cc: joao.pi...@synopsys.com; peppe.cavall...@st.com;
>> seraphin.bonna...@st.com; f.faine...@gmail.com;
>> alexandre.tor...@gmail.com; manab...@gmail.com; niklas.cas...@axis.com;
>> jo...@kernel.org; pa...@ucw.cz; Ong, Boon Leong
>> <boon.leong@intel.com>; Voon, Weifeng <weifeng.v...@intel.com>;
>> lars.pers...@axis.com; netdev@vger.kernel.org; linux-ker...@vger.kernel.org
>> Subject: Re: [PATCH v2] net: stmmac: bug fix to synchronize stmmac_open and
>> stmmac_dvr_probe
>>
>> From: "Kweh, Hock Leong" <hock.leong.k...@intel.com>
>> Date: Tue, 27 Dec 2016 22:42:36 +0800
>>
>>> From: "Kweh, Hock Leong" <hock.leong.k...@intel.com>
>>
>> You are not the author of this change, do not take credit for it.
>>
>> You have copied Florian's patch character by character, therefore
>> he is the author.
>>
>> You also didn't CC: the netdev mailing list properly.
> 
> Hi David & Florian,
> 
> Just to clarify that I do not copy exactly from Florian.
> I have changed it to have proper handling on mdio unregister
> while netdev_register() failed as showed below:
> 
>   return 0;
>  
> -error_mdio_register:
> - unregister_netdev(ndev);
>  error_netdev_register:
> + stmmac_mdio_unregister(ndev);

Although this is required, we can't be doing it in all circumstances, we
need to mimic what stmmac_drv_remove() does.

Let me submit an incremental fix which takes care of mdio bus
unregistration.
-- 
Florian

Re: [PATCH v2] net: stmmac: bug fix to synchronize stmmac_open and stmmac_dvr_probe

[Kishan Sandeep]

On Wed, Dec 28, 2016 at 7:10 AM, Kweh, Hock Leong
<hock.leong.k...@intel.com> wrote:
>> -Original Message-
>> From: David Miller [mailto:da...@davemloft.net]
>> Sent: Wednesday, December 28, 2016 12:34 AM
>> To: Kweh, Hock Leong <hock.leong.k...@intel.com>
>> Cc: joao.pi...@synopsys.com; peppe.cavall...@st.com;
>> seraphin.bonna...@st.com; f.faine...@gmail.com;
>> alexandre.tor...@gmail.com; manab...@gmail.com; niklas.cas...@axis.com;
>> jo...@kernel.org; pa...@ucw.cz; Ong, Boon Leong
>> <boon.leong@intel.com>; Voon, Weifeng <weifeng.v...@intel.com>;
>> lars.pers...@axis.com; netdev@vger.kernel.org; linux-ker...@vger.kernel.org
>> Subject: Re: [PATCH v2] net: stmmac: bug fix to synchronize stmmac_open and
>> stmmac_dvr_probe
>>
>> From: "Kweh, Hock Leong"  <hock.leong.k...@intel.com>
>> Date: Tue, 27 Dec 2016 22:42:36 +0800
>>
>> > From: "Kweh, Hock Leong" <hock.leong.k...@intel.com>
>>
>> You are not the author of this change, do not take credit for it.
>>
>> You have copied Florian's patch character by character, therefore
>> he is the author.
>>
>> You also didn't CC: the netdev mailing list properly.
>
> Noted & Thanks.
>
> Hi Florian, could you submit this fix from your side so that you are the 
> author.
> I will help to test out.
>
> Thanks & Regards,
> Wilson
>
I think you can give *--author* for giving author name. Try git commit
-am "commit message" --author="Author_name "

RE: [PATCH v2] net: stmmac: bug fix to synchronize stmmac_open and stmmac_dvr_probe

[Kweh, Hock Leong]

> -Original Message-
> From: David Miller [mailto:da...@davemloft.net]
> Sent: Wednesday, December 28, 2016 12:34 AM
> To: Kweh, Hock Leong <hock.leong.k...@intel.com>
> Cc: joao.pi...@synopsys.com; peppe.cavall...@st.com;
> seraphin.bonna...@st.com; f.faine...@gmail.com;
> alexandre.tor...@gmail.com; manab...@gmail.com; niklas.cas...@axis.com;
> jo...@kernel.org; pa...@ucw.cz; Ong, Boon Leong
> <boon.leong@intel.com>; Voon, Weifeng <weifeng.v...@intel.com>;
> lars.pers...@axis.com; netdev@vger.kernel.org; linux-ker...@vger.kernel.org
> Subject: Re: [PATCH v2] net: stmmac: bug fix to synchronize stmmac_open and
> stmmac_dvr_probe
> 
> From: "Kweh, Hock Leong"  <hock.leong.k...@intel.com>
> Date: Tue, 27 Dec 2016 22:42:36 +0800
> 
> > From: "Kweh, Hock Leong" <hock.leong.k...@intel.com>
> 
> You are not the author of this change, do not take credit for it.
> 
> You have copied Florian's patch character by character, therefore
> he is the author.
> 
> You also didn't CC: the netdev mailing list properly.

Hi David & Florian,

Just to clarify that I do not copy exactly from Florian.
I have changed it to have proper handling on mdio unregister
while netdev_register() failed as showed below:

return 0;
 
-error_mdio_register:
-   unregister_netdev(ndev);
 error_netdev_register:
+   stmmac_mdio_unregister(ndev);
+error_mdio_register:
netif_napi_del(>napi);

Vs 

+
+   return ret;

 error_mdio_register:
-   unregister_netdev(ndev);
-error_netdev_register:
netif_napi_del(>napi);

Just to point it out here, so that Florian could aware if he/she submit
this fix to the mailing list.


Thanks & Regards,
Wilson  

RE: [PATCH v2] net: stmmac: bug fix to synchronize stmmac_open and stmmac_dvr_probe

[Kweh, Hock Leong]

> -Original Message-
> From: David Miller [mailto:da...@davemloft.net]
> Sent: Wednesday, December 28, 2016 12:34 AM
> To: Kweh, Hock Leong <hock.leong.k...@intel.com>
> Cc: joao.pi...@synopsys.com; peppe.cavall...@st.com;
> seraphin.bonna...@st.com; f.faine...@gmail.com;
> alexandre.tor...@gmail.com; manab...@gmail.com; niklas.cas...@axis.com;
> jo...@kernel.org; pa...@ucw.cz; Ong, Boon Leong
> <boon.leong@intel.com>; Voon, Weifeng <weifeng.v...@intel.com>;
> lars.pers...@axis.com; netdev@vger.kernel.org; linux-ker...@vger.kernel.org
> Subject: Re: [PATCH v2] net: stmmac: bug fix to synchronize stmmac_open and
> stmmac_dvr_probe
> 
> From: "Kweh, Hock Leong"  <hock.leong.k...@intel.com>
> Date: Tue, 27 Dec 2016 22:42:36 +0800
> 
> > From: "Kweh, Hock Leong" <hock.leong.k...@intel.com>
> 
> You are not the author of this change, do not take credit for it.
> 
> You have copied Florian's patch character by character, therefore
> he is the author.
> 
> You also didn't CC: the netdev mailing list properly.

Noted & Thanks.

Hi Florian, could you submit this fix from your side so that you are the author.
I will help to test out.

Thanks & Regards,
Wilson

Re: [PATCH v2] net: stmmac: bug fix to synchronize stmmac_open and stmmac_dvr_probe

[David Miller]

From: "Kweh, Hock Leong"
Date: Tue, 27 Dec 2016 22:42:36 +0800

> From: "Kweh, Hock Leong" 

You are not the author of this change, do not take credit for it.

You have copied Florian's patch character by character, therefore
he is the author.

You also didn't CC: the netdev mailing list properly.

Re: [PATCH v2] net: stmmac: bug fix to synchronize stmmac_open and stmmac_dvr_probe

[Pavel Machek]

On Tue 2016-12-27 22:42:36, Kweh, Hock Leong wrote:
> From: "Kweh, Hock Leong" 
> 
> If kernel module stmmac driver being loaded after OS booted, there is a
> race condition between stmmac_open() and stmmac_mdio_register(), which is
> invoked inside stmmac_dvr_probe(), and the error is showed in dmesg log as
> PHY not found and stmmac_open() failed:
> [  473.919358] stmmaceth :01:00.0 (unnamed net_device) (uninitialized):
> stmmac_dvr_probe: warning: cannot get CSR clock
> [  473.919382] stmmaceth :01:00.0: no reset control found
> [  473.919412] stmmac - user ID: 0x10, Synopsys ID: 0x42
> [  473.919429] stmmaceth :01:00.0: DMA HW capability register supported
> [  473.919436] stmmaceth :01:00.0: RX Checksum Offload Engine supported
> [  473.919443] stmmaceth :01:00.0: TX Checksum insertion supported
> [  473.919451] stmmaceth :01:00.0 (unnamed net_device) (uninitialized):
> Enable RX Mitigation via HW Watchdog Timer
> [  473.921395] libphy: PHY stmmac-1:00 not found
> [  473.921417] stmmaceth :01:00.0 eth0: Could not attach to PHY
> [  473.921427] stmmaceth :01:00.0 eth0: stmmac_open: Cannot attach to
> PHY (error: -19)
> [  473.959710] libphy: stmmac: probed
> [  473.959724] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 0 IRQ POLL
> (stmmac-1:00) active
> [  473.959728] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 1 IRQ POLL
> (stmmac-1:01)
> [  473.959731] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 2 IRQ POLL
> (stmmac-1:02)
> [  473.959734] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 3 IRQ POLL
> (stmmac-1:03)
> 
> The resolution moved the register_netdev() function call to the end of
> stmmac_dvr_probe() after stmmac_mdio_register().
> 
> Suggested-by: Florian Fainelli 
> Signed-off-by: Kweh, Hock Leong 

Acked-by: Pavel Machek 

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature