Re: [PATCH v8] clk: Add (devm_)clk_get_optional() functions

[Stephen Boyd]

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;
> +}
> +