| Message ID | 20211224021500.10362-1-linmq006@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | b45396afa4177f2b1ddfeff7185da733fade1dc3 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: phy: fixed_phy: Fix NULL vs IS_ERR() checking in __fixed_phy_register | expand |
Hello: This patch was applied to netdev/net.git (master) by Jakub Kicinski <kuba@kernel.org>: On Fri, 24 Dec 2021 02:14:59 +0000 you wrote: > The fixed_phy_get_gpiod function() returns NULL, it doesn't return error > pointers, using NULL checking to fix this.i > > Fixes: 5468e82f7034 ("net: phy: fixed-phy: Drop GPIO from fixed_phy_add()") > Signed-off-by: Miaoqian Lin <linmq006@gmail.com> > --- > drivers/net/phy/fixed_phy.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Here is the summary with links: - net: phy: fixed_phy: Fix NULL vs IS_ERR() checking in __fixed_phy_register https://git.kernel.org/netdev/net/c/b45396afa417 You are awesome, thank you!
On Sun, Jan 02, 2022 at 11:43:13AM +0100, Sam Ravnborg wrote: > Hi Miaoqian, > > On Fri, Dec 24, 2021 at 02:14:59AM +0000, Miaoqian Lin wrote: > > The fixed_phy_get_gpiod function() returns NULL, it doesn't return error > > pointers, using NULL checking to fix this.i > > > > Fixes: 5468e82f7034 ("net: phy: fixed-phy: Drop GPIO from fixed_phy_add()") > > Signed-off-by: Miaoqian Lin <linmq006@gmail.com> > > This patch has the side-effect that the node <link-gpios> is now > mandatory. As the DT file I use do not have this node there is no > network. > The error I see in the log is: > > fec 2188000.ethernet: broken fixed-link specification > fec: probe of 2188000.ethernet failed with error -22 > > Reverting this patch gave me network again. > This is on top of 8008293888188c3923f5bd8a69370dae25ed14e5. > > Note: I have issues with my mail provider, so this mail may not hit the > mailing list. I think the real problem is: commit b45396afa4177f2b1ddfeff7185da733fade1dc3 Author: Miaoqian Lin <linmq006@gmail.com> Date: Fri Dec 24 02:14:59 2021 +0000 net: phy: fixed_phy: Fix NULL vs IS_ERR() checking in __fixed_phy_register The fixed_phy_get_gpiod function() returns NULL, it doesn't return error pointers, using NULL checking to fix this.i The phy_get_gpiod function() does in fact return an error code pointer for one important case: gpiod = fwnode_gpiod_get_index(of_fwnode_handle(fixed_link_node), "link", 0, GPIOD_IN, "mdio"); if (IS_ERR(gpiod) && PTR_ERR(gpiod) != -EPROBE_DEFER) { if (PTR_ERR(gpiod) != -ENOENT) pr_err("error getting GPIO for fixed link %pOF, proceed without\n", fixed_link_node); gpiod = NULL; } of_node_put(fixed_link_node); return gpiod; So if fwnode_gpiod_get_index() returns -EPROBE_DEFFER, and error code pointer is returned. And that error code pointer needs to be returned up the call stack in order that the driver gets probed again later when the GPIO driver has loaded. Miaoqian, please could you submit a fix for this. Andrew
diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c index c65fb5f5d2dc..a0c256bd5441 100644 --- a/drivers/net/phy/fixed_phy.c +++ b/drivers/net/phy/fixed_phy.c @@ -239,8 +239,8 @@ static struct phy_device *__fixed_phy_register(unsigned int irq, /* Check if we have a GPIO associated with this fixed phy */ if (!gpiod) { gpiod = fixed_phy_get_gpiod(np); - if (IS_ERR(gpiod)) - return ERR_CAST(gpiod); + if (!gpiod) + return ERR_PTR(-EINVAL); } /* Get the next available PHY address, up to PHY_MAX_ADDR */
The fixed_phy_get_gpiod function() returns NULL, it doesn't return error pointers, using NULL checking to fix this.i Fixes: 5468e82f7034 ("net: phy: fixed-phy: Drop GPIO from fixed_phy_add()") Signed-off-by: Miaoqian Lin <linmq006@gmail.com> --- drivers/net/phy/fixed_phy.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)