[PATCH v2 net] net: mdio: don't defer probe forever if PHY IRQ provider is missing

[PATCH v2 net] net: mdio: don't defer probe forever if PHY IRQ provider is missing

[Vladimir Oltean]

When a driver for an interrupt controller is missing, of_irq_get()
returns -EPROBE_DEFER ad infinitum, causing
fwnode_mdiobus_phy_device_register(), and ultimately, the entire
of_mdiobus_register() call, to fail. In turn, any phy_connect() call
towards a PHY on this MDIO bus will also fail.

This is not what is expected to happen, because the PHY library falls
back to poll mode when of_irq_get() returns a hard error code, and the
MDIO bus, PHY and attached Ethernet controller work fine, albeit
suboptimally, when the PHY library polls for link status. However,
-EPROBE_DEFER has special handling given the assumption that at some
point probe deferral will stop, and the driver for the supplier will
kick in and create the IRQ domain.

Reasons for which the interrupt controller may be missing:

- It is not yet written. This may happen if a more recent DT blob (with
  an interrupt-parent for the PHY) is used to boot an old kernel where
  the driver didn't exist, and that kernel worked with the
  vintage-correct DT blob using poll mode.

- It is compiled out. Behavior is the same as above.

- It is compiled as a module. The kernel will wait for a number of
  seconds specified in the "deferred_probe_timeout" boot parameter for
  user space to load the required module. The current default is 0,
  which times out at the end of initcalls. It is possible that this
  might cause regressions unless users adjust this boot parameter.

The proposed solution is to use the driver_deferred_probe_check_state()
helper function provided by the driver core, which gives up after some
-EPROBE_DEFER attempts, taking "deferred_probe_timeout" into consideration.
The return code is changed from -EPROBE_DEFER into -ENODEV or
-ETIMEDOUT, depending on whether the kernel is compiled with support for
modules or not.

Fixes: 66bdede495c7 ("of_mdio: Fix broken PHY IRQ in case of probe deferral")
Suggested-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v2: export driver_deferred_probe_check_state, add driver core
        maintainers

This will not apply to stable kernels prior to commit bc1bee3b87ee
("net: mdiobus: Introduce fwnode_mdiobus_register_phy()"). I am planning
to send an adapted patch for those when Greg sends out the emails
stating that the patch fails to apply.

 drivers/base/dd.c              | 1 +
 drivers/net/mdio/fwnode_mdio.c | 5 +++++
 2 files changed, 6 insertions(+)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index af6bea56f4e2..3fc3b5940bb3 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -296,6 +296,7 @@ int driver_deferred_probe_check_state(struct device *dev)
 
 	return -EPROBE_DEFER;
 }
+EXPORT_SYMBOL_GPL(driver_deferred_probe_check_state);
 
 static void deferred_probe_timeout_work_func(struct work_struct *work)
 {
diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
index 1becb1a731f6..1c1584fca632 100644
--- a/drivers/net/mdio/fwnode_mdio.c
+++ b/drivers/net/mdio/fwnode_mdio.c
@@ -43,6 +43,11 @@ int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio,
 	int rc;
 
 	rc = fwnode_irq_get(child, 0);
+	/* Don't wait forever if the IRQ provider doesn't become available,
+	 * just fall back to poll mode
+	 */
+	if (rc == -EPROBE_DEFER)
+		rc = driver_deferred_probe_check_state(&phy->mdio.dev);
 	if (rc == -EPROBE_DEFER)
 		return rc;
 
-- 
2.25.1

Re: [PATCH v2 net] net: mdio: don't defer probe forever if PHY IRQ provider is missing

[Greg Kroah-Hartman]

On Thu, Apr 07, 2022 at 07:55:38PM +0300, Vladimir Oltean wrote:
> When a driver for an interrupt controller is missing, of_irq_get()
> returns -EPROBE_DEFER ad infinitum, causing
> fwnode_mdiobus_phy_device_register(), and ultimately, the entire
> of_mdiobus_register() call, to fail. In turn, any phy_connect() call
> towards a PHY on this MDIO bus will also fail.
> 
> This is not what is expected to happen, because the PHY library falls
> back to poll mode when of_irq_get() returns a hard error code, and the
> MDIO bus, PHY and attached Ethernet controller work fine, albeit
> suboptimally, when the PHY library polls for link status. However,
> -EPROBE_DEFER has special handling given the assumption that at some
> point probe deferral will stop, and the driver for the supplier will
> kick in and create the IRQ domain.
> 
> Reasons for which the interrupt controller may be missing:
> 
> - It is not yet written. This may happen if a more recent DT blob (with
>   an interrupt-parent for the PHY) is used to boot an old kernel where
>   the driver didn't exist, and that kernel worked with the
>   vintage-correct DT blob using poll mode.
> 
> - It is compiled out. Behavior is the same as above.
> 
> - It is compiled as a module. The kernel will wait for a number of
>   seconds specified in the "deferred_probe_timeout" boot parameter for
>   user space to load the required module. The current default is 0,
>   which times out at the end of initcalls. It is possible that this
>   might cause regressions unless users adjust this boot parameter.
> 
> The proposed solution is to use the driver_deferred_probe_check_state()
> helper function provided by the driver core, which gives up after some
> -EPROBE_DEFER attempts, taking "deferred_probe_timeout" into consideration.
> The return code is changed from -EPROBE_DEFER into -ENODEV or
> -ETIMEDOUT, depending on whether the kernel is compiled with support for
> modules or not.
> 
> Fixes: 66bdede495c7 ("of_mdio: Fix broken PHY IRQ in case of probe deferral")
> Suggested-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> v1->v2: export driver_deferred_probe_check_state, add driver core
>         maintainers

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH v2 net] net: mdio: don't defer probe forever if PHY IRQ provider is missing

[Florian Fainelli]

On 4/7/22 09:55, Vladimir Oltean wrote:
> When a driver for an interrupt controller is missing, of_irq_get()
> returns -EPROBE_DEFER ad infinitum, causing
> fwnode_mdiobus_phy_device_register(), and ultimately, the entire
> of_mdiobus_register() call, to fail. In turn, any phy_connect() call
> towards a PHY on this MDIO bus will also fail.
> 
> This is not what is expected to happen, because the PHY library falls
> back to poll mode when of_irq_get() returns a hard error code, and the
> MDIO bus, PHY and attached Ethernet controller work fine, albeit
> suboptimally, when the PHY library polls for link status. However,
> -EPROBE_DEFER has special handling given the assumption that at some
> point probe deferral will stop, and the driver for the supplier will
> kick in and create the IRQ domain.
> 
> Reasons for which the interrupt controller may be missing:
> 
> - It is not yet written. This may happen if a more recent DT blob (with
>    an interrupt-parent for the PHY) is used to boot an old kernel where
>    the driver didn't exist, and that kernel worked with the
>    vintage-correct DT blob using poll mode.
> 
> - It is compiled out. Behavior is the same as above.
> 
> - It is compiled as a module. The kernel will wait for a number of
>    seconds specified in the "deferred_probe_timeout" boot parameter for
>    user space to load the required module. The current default is 0,
>    which times out at the end of initcalls. It is possible that this
>    might cause regressions unless users adjust this boot parameter.
> 
> The proposed solution is to use the driver_deferred_probe_check_state()
> helper function provided by the driver core, which gives up after some
> -EPROBE_DEFER attempts, taking "deferred_probe_timeout" into consideration.
> The return code is changed from -EPROBE_DEFER into -ENODEV or
> -ETIMEDOUT, depending on whether the kernel is compiled with support for
> modules or not.
> 
> Fixes: 66bdede495c7 ("of_mdio: Fix broken PHY IRQ in case of probe deferral")
> Suggested-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH v2 net] net: mdio: don't defer probe forever if PHY IRQ provider is missing

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Thu,  7 Apr 2022 19:55:38 +0300 you wrote:
> When a driver for an interrupt controller is missing, of_irq_get()
> returns -EPROBE_DEFER ad infinitum, causing
> fwnode_mdiobus_phy_device_register(), and ultimately, the entire
> of_mdiobus_register() call, to fail. In turn, any phy_connect() call
> towards a PHY on this MDIO bus will also fail.
> 
> This is not what is expected to happen, because the PHY library falls
> back to poll mode when of_irq_get() returns a hard error code, and the
> MDIO bus, PHY and attached Ethernet controller work fine, albeit
> suboptimally, when the PHY library polls for link status. However,
> -EPROBE_DEFER has special handling given the assumption that at some
> point probe deferral will stop, and the driver for the supplier will
> kick in and create the IRQ domain.
> 
> [...]

Here is the summary with links:
  - [v2,net] net: mdio: don't defer probe forever if PHY IRQ provider is missing
    https://git.kernel.org/netdev/net/c/74befa447e68

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html