[U-Boot] [PATCH] ns16650: Make sure we have CONFIG_CLK set before using infrastructure
Masahiro Yamada
yamada.masahiro at socionext.com
Fri Sep 23 04:27:25 CEST 2016
Hi Tom,
2016-09-23 10:11 GMT+09:00 Tom Rini <trini at 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 at 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.
--
Best Regards
Masahiro Yamada
More information about the U-Boot
mailing list