Hi Simon,
2016-01-21 11:46 GMT+09:00 Simon Glass sjg@chromium.org:
Hi Masahiro,
On 20 January 2016 at 19:26, Masahiro Yamada yamada.masahiro@socionext.com wrote:
Hi Simon,
2016-01-21 0:24 GMT+09:00 Simon Glass sjg@chromium.org:
Hi Masahiro,
On 19 January 2016 at 22:38, Masahiro Yamada yamada.masahiro@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.
The only useful case I've come up with is when OF_CONTROL is optional.
ret = clk_get_by_index() if (ret == -ENOSYS) { If OF_CONTROL is disabled, try best effort to do something ... }