[U-Boot] ns16650: Make sure we have CONFIG_CLK set before using infrastructure

We cannot call on the CONFIG_CLK based clk_get_rate function unless
CONFIG_CLK is set.

Signed-off-by: Tom Rini <trini@konsulko.com>
---
 drivers/serial/ns16550.c |  7 +++++--
 include/clk.h            | 25 ++++++++++++-------------
 2 files changed, 17 insertions(+), 15 deletions(-)

On Thu, Sep 22, 2016 at 09:11:22PM -0400, Tom Rini wrote:

> We cannot call on the CONFIG_CLK based clk_get_rate function unless
> CONFIG_CLK is set.
> 
> Signed-off-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!

Hi Tom,


2016-09-23 10:11 GMT+09:00 Tom Rini <trini@konsulko.com>:
> We cannot call on the CONFIG_CLK based clk_get_rate function unless
> CONFIG_CLK is set.
>
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
>  drivers/serial/ns16550.c |  7 +++++--
>  include/clk.h            | 25 ++++++++++++-------------
>  2 files changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
> index 3f6ea4d27591..765499dab646 100644
> --- a/drivers/serial/ns16550.c
> +++ b/drivers/serial/ns16550.c
> @@ -13,6 +13,7 @@
>  #include <serial.h>
>  #include <watchdog.h>
>  #include <linux/types.h>
> +#include <linux/compiler.h>
>  #include <asm/io.h>
>
>  DECLARE_GLOBAL_DATA_PTR;
> @@ -353,8 +354,8 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
>  {
>         struct ns16550_platdata *plat = dev->platdata;
>         fdt_addr_t addr;
> -       struct clk clk;
> -       int err;
> +       __maybe_unused struct clk clk;
> +       __maybe_unused int err;
>
>         /* try Processor Local Bus device first */
>         addr = dev_get_addr(dev);
> @@ -401,6 +402,7 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
>         plat->reg_shift = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
>                                          "reg-shift", 0);
>
> +#ifdef CONFIG_CLK
>         err = clk_get_by_index(dev, 0, &clk);
>         if (!err) {
>                 err = clk_get_rate(&clk);
> @@ -410,6 +412,7 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
>                 debug("ns16550 failed to get clock\n");
>                 return err;
>         }
> +#endif
>
>         if (!plat->clock)
>                 plat->clock = fdtdec_get_int(gd->fdt_blob, dev->of_offset,



Why did you need this patch?


If CONFIG_CLK is not set, clk_get_by_index() just returns -ENOSYS.

So, the compiler optimization will drop
the whole of if () ... else if ().


     err = clk_get_by_index(dev, 0, &clk);
     if (!err) {
             err = clk_get_rate(&clk);
             if (!IS_ERR_VALUE(err))
                     plat->clock = err;
     } else if (err != -ENODEV && err != -ENOSYS) {
             debug("ns16550 failed to get clock\n");
             return err;
     }


Why are you belt-and-braces guarding it with #ifdef CONFIG_CLK?




> diff --git a/include/clk.h b/include/clk.h
> index 94c003714700..7273127bb4fb 100644
> --- a/include/clk.h
> +++ b/include/clk.h
> @@ -98,19 +98,6 @@ int clk_get_by_index(struct udevice *dev, int index, struct clk *clk);
>   * @return 0 if OK, or a negative error code.
>   */
>  int clk_get_by_name(struct udevice *dev, const char *name, struct clk *clk);
> -#else
> -static inline int clk_get_by_index(struct udevice *dev, int index,
> -                                  struct clk *clk)
> -{
> -       return -ENOSYS;
> -}
> -
> -static inline int clk_get_by_name(struct udevice *dev, const char *name,
> -                          struct clk *clk)
> -{
> -       return -ENOSYS;
> -}
> -#endif
>
>  /**
>   * clk_request - Request a clock by provider-specific ID.
> @@ -175,5 +162,17 @@ int clk_enable(struct clk *clk);
>  int clk_disable(struct clk *clk);
>
>  int soc_clk_dump(void);
> +#else
> +static inline int clk_get_by_index(struct udevice *dev, int index,
> +                                  struct clk *clk)
> +{
> +       return -ENOSYS;
> +}
>
> +static inline int clk_get_by_name(struct udevice *dev, const char *name,
> +                          struct clk *clk)
> +{
> +       return -ENOSYS;
> +}
> +#endif
>  #endif


This just made the situation worse; it will cause
implicit declaration of clk_enable(), clk_set_rate(), etc
unless we patch clk consumers around with #ifdef CONFIG_CLK.

On Fri, Sep 23, 2016 at 11:27:25AM +0900, Masahiro Yamada wrote:
> Hi Tom,
> 
> 
> 2016-09-23 10:11 GMT+09:00 Tom Rini <trini@konsulko.com>:
> > We cannot call on the CONFIG_CLK based clk_get_rate function unless
> > CONFIG_CLK is set.
> >
> > Signed-off-by: Tom Rini <trini@konsulko.com>
> > ---
> >  drivers/serial/ns16550.c |  7 +++++--
> >  include/clk.h            | 25 ++++++++++++-------------
> >  2 files changed, 17 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
> > index 3f6ea4d27591..765499dab646 100644
> > --- a/drivers/serial/ns16550.c
> > +++ b/drivers/serial/ns16550.c
> > @@ -13,6 +13,7 @@
> >  #include <serial.h>
> >  #include <watchdog.h>
> >  #include <linux/types.h>
> > +#include <linux/compiler.h>
> >  #include <asm/io.h>
> >
> >  DECLARE_GLOBAL_DATA_PTR;
> > @@ -353,8 +354,8 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
> >  {
> >         struct ns16550_platdata *plat = dev->platdata;
> >         fdt_addr_t addr;
> > -       struct clk clk;
> > -       int err;
> > +       __maybe_unused struct clk clk;
> > +       __maybe_unused int err;
> >
> >         /* try Processor Local Bus device first */
> >         addr = dev_get_addr(dev);
> > @@ -401,6 +402,7 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
> >         plat->reg_shift = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
> >                                          "reg-shift", 0);
> >
> > +#ifdef CONFIG_CLK
> >         err = clk_get_by_index(dev, 0, &clk);
> >         if (!err) {
> >                 err = clk_get_rate(&clk);
> > @@ -410,6 +412,7 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
> >                 debug("ns16550 failed to get clock\n");
> >                 return err;
> >         }
> > +#endif
> >
> >         if (!plat->clock)
> >                 plat->clock = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
> 
> 
> 
> Why did you need this patch?

Because we have non-CONFIG_CLK clk_get_rate() functions, so we get
function prototype mismatches.

> > diff --git a/include/clk.h b/include/clk.h
> > index 94c003714700..7273127bb4fb 100644
> > --- a/include/clk.h
> > +++ b/include/clk.h
> > @@ -98,19 +98,6 @@ int clk_get_by_index(struct udevice *dev, int index, struct clk *clk);
> >   * @return 0 if OK, or a negative error code.
> >   */
> >  int clk_get_by_name(struct udevice *dev, const char *name, struct clk *clk);
> > -#else
> > -static inline int clk_get_by_index(struct udevice *dev, int index,
> > -                                  struct clk *clk)
> > -{
> > -       return -ENOSYS;
> > -}
> > -
> > -static inline int clk_get_by_name(struct udevice *dev, const char *name,
> > -                          struct clk *clk)
> > -{
> > -       return -ENOSYS;
> > -}
> > -#endif
> >
> >  /**
> >   * clk_request - Request a clock by provider-specific ID.
> > @@ -175,5 +162,17 @@ int clk_enable(struct clk *clk);
> >  int clk_disable(struct clk *clk);
> >
> >  int soc_clk_dump(void);
> > +#else
> > +static inline int clk_get_by_index(struct udevice *dev, int index,
> > +                                  struct clk *clk)
> > +{
> > +       return -ENOSYS;
> > +}
> >
> > +static inline int clk_get_by_name(struct udevice *dev, const char *name,
> > +                          struct clk *clk)
> > +{
> > +       return -ENOSYS;
> > +}
> > +#endif
> >  #endif
> 
> 
> This just made the situation worse; it will cause
> implicit declaration of clk_enable(), clk_set_rate(), etc
> unless we patch clk consumers around with #ifdef CONFIG_CLK.

There's none today, and I think it makes it more obvious when we have a
clock "framework" that's not using CONFIG_CLK and needs to be moved
over.

Without this k2*evm fails to build.