Hi Ahmad,
On 24-02-16, Ahmad Fatoum wrote:
> -ENODEV is a bad choice for an error code for of_device_ensure_probed.
>
> The function is either called from board code or from driver frameworks,
> which usually just propagate the error code with unintended
> consequences:
>
> - A board driver probe function returning -ENODEV is silently skipped
>
> - A driver framework function returning -ENODEV is often interpreted
> to mean that an optional resource is not specified (e.g. in DT).
>
> In both cases, the user isn't provided an error message and wrong
> behavior can crop up later. In my case, the XHCI driver would time out,
> because phy_get propagated of_device_ensure_probed's -ENODEV, which was
> understood to mean that no PHY is needed and not that the PHY is
> specified in the DT, but no driver was bound to it.
>
> Instead of -ENODEV, let's thus return -EPROBE_DEFER. This can be
> propagated up to the driver core, which on a deep probe system (the only
> ones where of_device_ensure_probed is not a no-op) will print a fat red
> error message. We could achieve the same with some other error code, but
This big fat error should indicate that something went really wrong.
> -EPROBE_DEFER is what a non-deep-probe system would return when probes
> are deferred indefinitely, so symmetry in the deep probe case fits well.
Albeit I get your point, I'm not very happy to return this error code
here since the whole idea of deep-probe was to get rid of EPROBE_DEFER.
> Signed-off-by: Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx>
> ---
> drivers/of/platform.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 060fa3458bd2..664af49d268f 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -504,7 +504,7 @@ int of_device_ensure_probed(struct device_node *np)
> * requirements are fulfilled if 'dev->driver' is not NULL.
> */
> if (!dev->driver)
> - return -ENODEV;
> + return -EPROBE_DEFER;
What about a new error code, e.g. ENODRV which is only used once in this
function? Additional we can add an error message like:
pr_err("No driver found for %pO\n, np); since we know that the driver is
missing.
If you want to stick with -EPROBE_DEFER you need at least adapt the
above comment and the function doc.
Regards,
Marco
>
> return 0;
> }
> --
> 2.39.2
>
>
>
