linux-kernel - Re: [PATCH v8] clk: Add (devm_)clk_get

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: <154356894146.88331.4169293505438588293@swboyd.mtv.corp.google.com>
Date:   Fri, 30 Nov 2018 01:09:01 -0800
From:   Stephen Boyd <sboyd@...nel.org>
To:     Michael Turquette <mturquette@...libre.com>,
        Phil Edworthy <phil.edworthy@...esas.com>,
        Russell King <linux@...linux.org.uk>
Cc:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Uwe Kleine-König <u.kleine-koenig@...gutronix.de>,
        linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        Phil Edworthy <phil.edworthy@...esas.com>
Subject: Re: [PATCH v8] clk: Add (devm_)clk_get_optional() functions

Quoting Phil Edworthy (2018-11-20 06:14:45)
> This adds clk_get_optional() and devm_clk_get_optional() functions to get
> optional clocks.
> They behave the same as (devm_)clk_get except where there is no clock
> producer. In this case, instead of returning -ENOENT, the function
> returns NULL. This makes error checking simpler and allows
> clk_prepare_enable, etc to be called on the returned reference
> without additional checks.

Ok. I guess that works by virtue of how -ENOENT is returned by various
functions that are called deeper in the clk_get() path? I'm cautiously
optimistic. So cautious, we should probably add a comment to these
optional functions that indicate they rely on the functions they call to
return -ENOENT under the various conditions that make a clk optional.

> 
> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index a7773b5c0b9f..3ea3c78f62dd 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -383,6 +383,17 @@ int __must_check devm_clk_bulk_get_all(struct device *dev,
>   */
>  struct clk *devm_clk_get(struct device *dev, const char *id);
>  
> +/**
> + * devm_clk_get_optional - lookup and obtain a managed reference to an optional
> + *                        clock producer.
> + * @dev: device for clock "consumer"
> + * @id: clock consumer ID
> + *
> + * Behaves the same as devm_clk_get except where there is no clock producer. In

Please add () around devm_clk_get() so we know it's a function.

> + * this case, instead of returning -ENOENT, the function returns NULL.
> + */
> +struct clk *devm_clk_get_optional(struct device *dev, const char *id);
> +
>  /**
>   * devm_get_clk_from_child - lookup and obtain a managed reference to a
>   *                          clock producer from child node.
> @@ -718,6 +729,12 @@ static inline struct clk *devm_clk_get(struct device *dev, const char *id)
>         return NULL;
>  }
>  
> +static inline struct clk *devm_clk_get_optional(struct device *dev,
> +                                               const char *id)
> +{
> +       return NULL;
> +}
> +
>  static inline int __must_check devm_clk_bulk_get(struct device *dev, int num_clks,
>                                                  struct clk_bulk_data *clks)
>  {
> @@ -862,6 +879,16 @@ static inline void clk_bulk_disable_unprepare(int num_clks,
>         clk_bulk_unprepare(num_clks, clks);
>  }
>  
> +static inline struct clk *clk_get_optional(struct device *dev, const char *id)

Any kernel doc for this function?

> +{
> +       struct clk *clk = clk_get(dev, id);
> +
> +       if (clk == ERR_PTR(-ENOENT))
> +               clk = NULL;
> +
> +       return clk;
> +}
> +