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

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:

quoted

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 <redacted>

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 [off-list ref]
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