linux-kernel - Re: [PATCH 04/10] clk: gpio: Simplify with dev_err

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <159977526650.2295844.8933765686946237404@swboyd.mtv.corp.google.com>
Date:   Thu, 10 Sep 2020 15:01:06 -0700
From:   Stephen Boyd <sboyd@...nel.org>
To:     Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Chen-Yu Tsai <wens@...e.org>,
        David Lechner <david@...hnology.com>,
        Jerome Brunet <jbrunet@...libre.com>,
        Kevin Hilman <khilman@...libre.com>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        Ludovic Desroches <ludovic.desroches@...rochip.com>,
        Martin Blumenstingl <martin.blumenstingl@...glemail.com>,
        Maxime Ripard <mripard@...nel.org>,
        Michael Turquette <mturquette@...libre.com>,
        Neil Armstrong <narmstrong@...libre.com>,
        Nicolas Ferre <nicolas.ferre@...rochip.com>,
        Santosh Shilimkar <ssantosh@...nel.org>,
        Sekhar Nori <nsekhar@...com>,
        linux-amlogic@...ts.infradead.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-arm-msm@...r.kernel.org, linux-clk@...r.kernel.org,
        linux-kernel@...r.kernel.org
Cc:     Krzysztof Kozlowski <krzk@...nel.org>
Subject: Re: [PATCH 04/10] clk: gpio: Simplify with dev_err_probe()

Quoting Krzysztof Kozlowski (2020-09-02 08:03:42)
> diff --git a/drivers/clk/clk-gpio.c b/drivers/clk/clk-gpio.c
> index 38755a241ab7..a3cc53edcb11 100644
> --- a/drivers/clk/clk-gpio.c
> +++ b/drivers/clk/clk-gpio.c
> @@ -211,17 +210,10 @@ static int gpio_clk_driver_probe(struct platform_device *pdev)
>  
>         gpio_name = is_mux ? "select" : "enable";
>         gpiod = devm_gpiod_get(dev, gpio_name, GPIOD_OUT_LOW);
> -       if (IS_ERR(gpiod)) {
> -               ret = PTR_ERR(gpiod);
> -               if (ret == -EPROBE_DEFER)
> -                       pr_debug("%pOFn: %s: GPIOs not yet available, retry later\n",
> -                                       node, __func__);
> -               else
> -                       pr_err("%pOFn: %s: Can't get '%s' named GPIO property\n",
> -                                       node, __func__,
> -                                       gpio_name);
> -               return ret;
> -       }
> +       if (IS_ERR(gpiod))
> +               return dev_err_probe(dev, PTR_ERR(gpiod),

This is cool! I wonder if we could make it even more simplified with

	ret = dev_err_probe_ptr(dev, ptr, ...)
	if (ret)
		return ret;

then we don't have to do the PTR_ERR() or IS_ERR() dance in all the
drivers. It could already be changed here to look at the return value of
dev_err_probe() so please do that at the least.

It would also even be more super duper cool if we had a way to save some
sort of cookie when the provider can't find it and is returning the
-EPROBE_DEFER value. Maybe the provider could use
device_set_deferred_probe_reason() on error and then if dev_err_probe()
is called without any string it can print what is in the device's
deferred probe reason? Or append to it whatever string is passed from
the device driver? Sometimes the provider has more info like the DT
property is malformed or the provider isn't probed yet which would
probably help understand the deferred problem more.