Hi Masahiro, On 20 January 2016 at 19:26, Masahiro Yamada <yamada.masah...@socionext.com> wrote: > Hi Simon, > > > 2016-01-21 0:24 GMT+09:00 Simon Glass <s...@chromium.org>: >> Hi Masahiro, >> >> On 19 January 2016 at 22:38, Masahiro Yamada >> <yamada.masah...@socionext.com> wrote: >>> >>> Hi Simon, >>> >>> >>> >>> >>> >>> +/** >>> >>> + * clk_get_by_index() - look up a clock referenced by a device >>> >>> + * >>> >>> + * Parse a device's 'clocks' list, returning information on the >>> >>> indexed clock, >>> >>> + * ensuring that it is activated. >>> >>> + * >>> >>> + * @dev: Device containing the clock reference >>> >>> + * @index: Clock index to return (0 = first) >>> >>> + * @clk_devp: Returns clock device >>> >>> + * @return: Peripheral ID for the device to control. This is the >>> >>> first >>> >>> + * argument after the clock node phandle. If there is no >>> >>> arguemnt, >>> >>> + * returns 0. Return -ve error code on any error >>> >>> + */ >>> >>> +int clk_get_by_index(struct udevice *dev, int index, struct udevice >>> >>> **clk_devp); >>> >>> #endif /* _CLK_H_ */ >>> >> >>> >> >>> >> I want #ifdef in the header too, like mine >>> >> http://patchwork.ozlabs.org/patch/566812/ >>> > >>> > I am not keen on that idea since it clutters up header files and we'll >>> > get a link error anyway if something is missing. Anyway, I've added >>> > it. >>> >>> >>> >>> I am afraid there is misunderstanding here. >>> >>> Please see my patch carefully. >>> >>> >>> What I mean is like this: >>> >>> >>> #if ... >>> declaration of function prototype >>> #else >>> static inline empty function >>> #endif >>> >>> >>> This is a common technique to avoid a link error. >> >> Do you think this function will be called when device tree is not >> enabled? I cannot see how it would have any meaning in that case. What >> is the purpose? > > > In order to avoid build errors. > > > I think this coding style is often used. > > > For example, you can see include/linux/clk.h in Linux. > > > #if defined(CONFIG_OF) && defined(CONFIG_COMMON_CLK) > struct clk *of_clk_get(struct device_node *np, int index); > struct clk *of_clk_get_by_name(struct device_node *np, const char *name); > struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec); > #else > static inline struct clk *of_clk_get(struct device_node *np, int index) > { > return ERR_PTR(-ENOENT); > } > static inline struct clk *of_clk_get_by_name(struct device_node *np, > const char *name) > { > return ERR_PTR(-ENOENT); > } > #endif > > > > > If every driver entry in Kconfig specifies "depends on OF_CONTROL" correctly, > no link error would happen for this. > > So, I leave this decision to you. > If you do not like #ifdef in header files, just rip it off.
I'm not a huge fan of this - I feel that a link error might be preferable to a run-time error. But I don't really have a strong opinion on it. I've sent a new patch. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot