[PATCH v5 1/5] dm: clk: add stub when CONFIG_CLK is deactivated

[Simon Goldschmidt]

On Fri, Mar 6, 2020 at 11:01 AM Patrick Delaunay
<patrick.delaunay at st.com> wrote:
>
> Add stub for functions clk_...() when CONFIG_CLK is deactivated.
>
> This patch avoids compilation issues for driver using these API
> without protection (#if CONFIG_IS_ENABLED(CLK))
>
> For example, before this patch we have undefined reference to
> `clk_disable_bulk') for code:
>   clk_disable_bulk(&priv->clks);
>   clk_release_bulk(&priv->clks);
>
> The bulk stub set and test bulk->count to avoid error for the sequence:
>
>   clk_get_bulk(dev, &priv->bulk);
>         ....
>   if (clk_enable(&priv>bulk))
>         return -EIO;
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>
> ---
>
> Changes in v5:
> - use ERR_PTR in clk_get_parent()
> - force bulk->count = 0 in clk_get_bulk to avoid issue
>   for next call of clk_enable_bulk / clk_enable_bulk

I wonder if this is correct. I think it only produces additional code. If you
always set bulk->count to 0, the enable code will never fail.

However, the enable function should never be executed as enable returns an
error. I can imagine the *disable* function could be called, but the return
value of this function is currently only handled in clk_sandbox_test.c...

Wouldn't it work to just remove all checks for bulk->count and let *all*
redefined functions return a constant (success or failure)? That would help
to keep the code small.

Looking at linux, clock disable functions have *no* return value that needs
to be checked, clock enable functions return success when compiled without
clock support.

Regards,
Simon

> - update commit message
>
> Changes in v4:
> - Add stub for all functions using 'struct clk' or 'struct clk_bulk'
>   after remarks on v3
>
> Changes in v3:
> - Add stub for clk_disable_bulk
>
> Changes in v2: None
>
>  include/clk.h | 104 +++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 91 insertions(+), 13 deletions(-)
>
> diff --git a/include/clk.h b/include/clk.h
> index 3336301815..ca8f1cfec7 100644
> --- a/include/clk.h
> +++ b/include/clk.h
> @@ -9,6 +9,7 @@
>  #define _CLK_H_
>
>  #include <dm/ofnode.h>
> +#include <linux/err.h>
>  #include <linux/errno.h>
>  #include <linux/types.h>
>
> @@ -249,6 +250,8 @@ static inline int clk_get_by_index(struct udevice *dev, int index,
>
>  static inline int clk_get_bulk(struct udevice *dev, struct clk_bulk *bulk)
>  {
> +       if (bulk)
> +               bulk->count = 0;
>         return -ENOSYS;
>  }
>
> @@ -312,6 +315,7 @@ static inline int clk_release_bulk(struct clk_bulk *bulk)
>         return clk_release_all(bulk->clks, bulk->count);
>  }
>
> +#if CONFIG_IS_ENABLED(CLK)
>  /**
>   * clk_request - Request a clock by provider-specific ID.
>   *
> @@ -433,19 +437,6 @@ int clk_disable_bulk(struct clk_bulk *bulk);
>   */
>  bool clk_is_match(const struct clk *p, const struct clk *q);
>
> -int soc_clk_dump(void);
> -
> -/**
> - * clk_valid() - check if clk is valid
> - *
> - * @clk:       the clock to check
> - * @return true if valid, or false
> - */
> -static inline bool clk_valid(struct clk *clk)
> -{
> -       return clk && !!clk->dev;
> -}
> -
>  /**
>   * clk_get_by_id() - Get the clock by its ID
>   *
> @@ -465,6 +456,93 @@ int clk_get_by_id(ulong id, struct clk **clkp);
>   * @return true on binded, or false on no
>   */
>  bool clk_dev_binded(struct clk *clk);
> +
> +#else /* CONFIG_IS_ENABLED(CLK) */
> +
> +static inline int clk_request(struct udevice *dev, struct clk *clk)
> +{
> +       return -ENOSYS;
> +}
> +
> +static inline int clk_free(struct clk *clk)
> +{
> +       return -ENOSYS;
> +}
> +
> +static inline ulong clk_get_rate(struct clk *clk)
> +{
> +       return -ENOSYS;
> +}
> +
> +static inline struct clk *clk_get_parent(struct clk *clk)
> +{
> +       return ERR_PTR(-ENOSYS);
> +}
> +
> +static inline long long clk_get_parent_rate(struct clk *clk)
> +{
> +       return -ENOSYS;
> +}
> +
> +static inline ulong clk_set_rate(struct clk *clk, ulong rate)
> +{
> +       return -ENOSYS;
> +}
> +
> +static inline int clk_set_parent(struct clk *clk, struct clk *parent)
> +{
> +       return -ENOSYS;
> +}
> +
> +static inline int clk_enable(struct clk *clk)
> +{
> +       return -ENOSYS;
> +}
> +
> +static inline int clk_enable_bulk(struct clk_bulk *bulk)
> +{
> +       return bulk && bulk->count == 0 ? 0 : -ENOSYS;
> +}
> +
> +static inline int clk_disable(struct clk *clk)
> +{
> +       return -ENOSYS;
> +}
> +
> +static inline int clk_disable_bulk(struct clk_bulk *bulk)
> +{
> +       return bulk && bulk->count == 0 ? 0 : -ENOSYS;
> +}
> +
> +static inline bool clk_is_match(const struct clk *p, const struct clk *q)
> +{
> +       return false;
> +}
> +
> +static inline int clk_get_by_id(ulong id, struct clk **clkp)
> +{
> +       return -ENOSYS;
> +}
> +
> +static inline bool clk_dev_binded(struct clk *clk)
> +{
> +       return false;
> +}
> +#endif /* CONFIG_IS_ENABLED(CLK) */
> +
> +/**
> + * clk_valid() - check if clk is valid
> + *
> + * @clk:       the clock to check
> + * @return true if valid, or false
> + */
> +static inline bool clk_valid(struct clk *clk)
> +{
> +       return clk && !!clk->dev;
> +}
> +
> +int soc_clk_dump(void);
> +
>  #endif
>
>  #define clk_prepare_enable(clk) clk_enable(clk)
> --
> 2.17.1
>