[U-Boot] [PATCH v2 01/50] dm: clk: Add support for decoding clocks from the device tree

[Masahiro Yamada]

Hi Simon,


2016-01-21 0:24 GMT+09:00 Simon Glass <sjg at chromium.org>:
> Hi Masahiro,
>
> On 19 January 2016 at 22:38, Masahiro Yamada
> <yamada.masahiro at 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.



-- 
Best Regards
Masahiro Yamada