Re: [PATCH] watchdog: s3c2410: Fix getting the optional clock

[Guenter Roeck <linux@roeck-us.net>]

On 12/12/21 9:02 AM, Sam Protsenko wrote:
> "watchdog_src" clock is optional and may not be present for some SoCs
> supported by this driver. Nevertheless, in case the clock is provided
> but some error happens during its getting, that error should be handled
> properly. Use devm_clk_get_optional() API for that. Also report possible
> errors using dev_err_probe() to handle properly -EPROBE_DEFER error (if
> clock provider is not ready by the time WDT probe function is executed).
> 
> Fixes: a4f3dc8d5fbc ("watchdog: s3c2410: Support separate source clock")
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Suggested-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   drivers/watchdog/s3c2410_wdt.c | 22 ++++++++++++----------
>   1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> index bb374b9fc163..71c280d3e1a2 100644
> --- a/drivers/watchdog/s3c2410_wdt.c
> +++ b/drivers/watchdog/s3c2410_wdt.c
> @@ -713,16 +713,18 @@ static int s3c2410wdt_probe(struct platform_device *pdev)
>   	 * "watchdog_src" clock is optional; if it's not present -- just skip it
>   	 * and use "watchdog" clock as both bus and source clock.
>   	 */
> -	wdt->src_clk = devm_clk_get(dev, "watchdog_src");
> -	if (!IS_ERR(wdt->src_clk)) {
> -		ret = clk_prepare_enable(wdt->src_clk);
> -		if (ret < 0) {
> -			dev_err(dev, "failed to enable source clock\n");
> -			ret = PTR_ERR(wdt->src_clk);
> -			goto err_bus_clk;
> -		}
> -	} else {
> -		wdt->src_clk = NULL;
> +	wdt->src_clk = devm_clk_get_optional(dev, "watchdog_src");
> +	if (IS_ERR(wdt->src_clk)) {
> +		dev_err_probe(dev, PTR_ERR(wdt->src_clk),
> +			      "failed to get source clock\n");
> +		ret = PTR_ERR(wdt->src_clk);
> +		goto err_bus_clk;
> +	}
> +
> +	ret = clk_prepare_enable(wdt->src_clk);
> +	if (ret) {
> +		dev_err(dev, "failed to enable source clock\n");
> +		goto err_bus_clk;
>   	}
>   
>   	wdt->wdt_device.min_timeout = 1;
>