[PATCH] net: phy: fixed_phy: Fix NULL vs IS_ERR() checking in __fixed_phy_register

[PATCH] net: phy: fixed_phy: Fix NULL vs IS_ERR() checking in __fixed_phy_register

[Miaoqian Lin]

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(-)

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 */
-- 
2.17.1

Re: [PATCH] net: phy: fixed_phy: Fix NULL vs IS_ERR() checking in __fixed_phy_register

[patchwork-bot+netdevbpf]

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!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: No network [Was: [PATCH] net: phy: fixed_phy: Fix NULL vs IS_ERR() checking in __fixed_phy_register]

[Andrew Lunn]

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