From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michal Simek Date: Wed, 18 May 2016 12:56:52 +0200 Subject: [U-Boot] [PATCH] test/py: Detect timeout in phy negotiation In-Reply-To: <573B6C7B.9070807@wwwdotorg.org> References: <0abce825b8b0d541148207594be5bb1512cbf40e.1463493444.git.michal.simek@xilinx.com> <573B4BBF.8090508@wwwdotorg.org> <573B4D58.4030007@xilinx.com> <573B4E26.7070909@wwwdotorg.org> <573B4ED6.2000607@xilinx.com> <573B56B6.1020600@wwwdotorg.org> <573B5C44.4060109@xilinx.com> <573B6C7B.9070807@wwwdotorg.org> Message-ID: <573C4A74.5070508@xilinx.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 17.5.2016 21:09, Stephen Warren wrote: > On 05/17/2016 12:00 PM, Michal Simek wrote: >> On 17.5.2016 19:36, Stephen Warren wrote: >>> On 05/17/2016 11:03 AM, Michal Simek wrote: >>>> On 17.5.2016 19:00, Stephen Warren wrote: >>>>> On 05/17/2016 10:56 AM, Michal Simek wrote: >>>>>> Hi Stephen, >>>>>> >>>>>> On 17.5.2016 18:50, Stephen Warren wrote: >>>>>>> On 05/17/2016 07:57 AM, Michal Simek wrote: >>>>>>>> If timeout happen it should be reported as fault. >>>>>>> >>>>>>> Presumably if a timeout occurs, the expected text does not appear, >>>>>>> i.e. >>>>>>> the existing assert fails anyway? >>>>>>> >>>>>>> Anyway, it's useful to point out problems explicitly, so, >>>>>>> Acked-by: Stephen Warren >>>>>> >>>>>> Unfortunately I found this issue when I was checking logs where I am >>>>>> getting this. >>>>>> >>>>>> ethernet at e000b000 Waiting for PHY auto negotiation to >>>>>> complete......... >>>>>> TIMEOUT ! >>>>>> BOOTP broadcast 1 >>>>>> BOOTP broadcast 2 >>>>>> BOOTP broadcast 3 >>>>>> DHCP client bound to address 192.168.0.107 (882 ms) >>>>>> Zynq> .Zynq> setenv serverip 192.168.0.105 >>>>>> >>>>>> I haven't looked at the exact reason why it is failing but IMHO it is >>>>>> worth to check. >>>>> >>>>> Oh, in that case I think I should withdraw my ack; in the log >>>>> above, the >>>>> operation completed successfully, so I'm not convinced the test should >>>>> be marked a failure. I thought this change simply provided more detail >>>>> re: the cause of a test failure. >>>> >>>> Is there any other way how to run just phy negotiation and mark this >>>> test as fail? >>> >>> I don't see anything obvious that will do that; I think that only >>> happens when net_loop starts. It might be possible to add some new >>> command to test just PHY startup, or a new mode for net_loop() that just >>> waited for link up and did no protocol work. >> >> ok. It means you are saying that this is bug and we are not able to test >> it by single test. That's why I think this should be report as a bug >> when this is happening just to hands up there is a problem. >> >> Not sure if there is any way to report is as a warning instead of bug >> that test case will pass with warning. > > At least my only-marginally-old version of pytest (2.5.1) doesn't appear > to have any concept of warnings. Some quite new versions of pytest do, > although I'm not sure how they're reported. Integrating some form of > warning system into the tests could be useful though. ok. > > I wonder if the correct fix for this issue isn't to make the PHY code > actively return an error whenever there is a TIMEOUT raised, rather than > presumably ignoring it as it does now (since the network code goes on to > actually work correctly)? I will send some patches to fix this problem that transfers not start when TIMEOUT happens. All these boards should be fixed to return error codes properly. drivers/net/cpsw.c:612: phy_startup(phy); drivers/net/keystone_net.c:581: phy_startup(phy_dev); drivers/net/keystone_net.c:768: phy_startup(priv->phydev); drivers/net/lpc32xx_eth.c:605: phy_startup(phydev); drivers/net/mvgbe.c:697: phy_startup(phydev); drivers/net/mvneta.c:1478: phy_startup(phydev); drivers/net/mvpp2.c:3384: phy_startup(phy_dev); drivers/net/pic32_eth.c:343: phy_startup(priv->phydev); But again if we have visible errors and code is not handling it properly I do agree that code should be fixed first but still testing should be able to capture this problem for others to avoid that this problem happens again. Thanks, Michal