On 10/9/20 5:15 PM, Jakub Kicinski wrote: > On Fri, 9 Oct 2020 09:20:30 +0200 Marek Vasut wrote: >>> Can you describe your problem in detail? >> >> Yes, I tried to do that in the commit message and the extra detailed >> comment above the code. What exactly do you not understand from that? > > Why it's not bound on the first open
It is getting bound on the first open. The problem is in probe(), where fec_enet_clk_enable(ndev, true) [yes, the name of that function is bad] calls fec_enet_phy_reset_after_clk_enable() and the ndev->phydev is NULL while there is already existing instance of that phydev . So this patch adds this extra look up to get the phydev, which then permits phy_reset_after_clk_enable() to call phy_device_reset() instead of returning -ENODEV. > (I'm guessing it's the first open > that has the ndev->phydev == NULL? I shouldn't have to guess). If I had a crystal ball that'd tell me all the review questions up front, I would write perfect patches with all the feedback sorted out in V1. Sorry, I don't have one ... >>> To an untrained eye this looks pretty weird. >> >> I see, I'm not quite sure how to address this comment. > > If ndev->phydev sometimes is not-NULL on open, then that's a valid > state to be in. Why not make sure that we're always in that state > and can depend on ndev->phydev rather than rummaging around for > the phy_device instance. Nope, the problem is in probe() in this case.
