Re: [PATCH RESEND V2 3/4] iio: frequency: adf4350: Add support for clock consumer framework

[Jonathan Cameron <jic23@kernel.org>]

On 06/06/2013 03:41 PM, Lars-Peter Clausen wrote:
> On 06/05/2013 10:18 PM, Jonathan Cameron wrote:
>> On 06/05/2013 07:07 PM, Lars-Peter Clausen wrote:
>>> On 06/05/2013 07:43 PM, Jonathan Cameron wrote:
>>>> On 06/05/2013 06:34 PM, Lars-Peter Clausen wrote:
>>>>> On 06/05/2013 05:25 PM, Jonathan Cameron wrote:
>>>>>> On 05/06/13 15:25, Lars-Peter Clausen wrote:
>>>>>>> On 06/05/2013 09:32 AM, Michael Hennerich wrote:
>>>>>>>> On 06/04/2013 07:44 PM, Jonathan Cameron wrote:
>>>>>>>>> On 06/03/2013 02:30 PM, michael.hennerich@analog.com wrote:
>>>>>>>>>> From: Michael Hennerich <michael.hennerich@analog.com>
>>>>>>>>>>
>>>>>>>>>> Preferably get clkin (PLL reference clock) from clock framework
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
>>>>>>>>> ERROR: "clk_round_rate" [drivers/iio/frequency/adf4350.ko] undefined!
>>>>>>>>> make[1]: *** [__modpost] Error 1
>>>>>>>>>
>>>>>>>>> on my arm test build. Sorry, I was being lazy before and hadn't done
>>>>>>>>> any test builds till I tried merging it.  Backed out the merge of this
>>>>>>>>> patch for now.
>>>>>>>>>
>>>>>>>>> clk.h does say that api is optional for machine classes.  No idea what you
>>>>>>>>> want to
>>>>>>>>> do about this.
>>>>>>>>
>>>>>>>> The CLK API is optional in the driver - so I prefer to keep it like that and
>>>>>>>> don't make
>>>>>>>> the driver depend on COMMON_CLK.
>>>>>>>>
>>>>>>>> The simplest and probably the best workaround is to guard the CLK Round/Set
>>>>>>>> code with
>>>>>>>> #if defined(CONFIG_COMMON_CLK).
>>>>>>>>
>>>>>>>> Thoughts?
>>>>>>>>
>>>>>>>
>>>>>>> This is not a bug in the driver. If the clk API is not implemented it is
>>>>>>> stubbed out. It looks as if Jonathan is testing on a platform which
>>>>>>> implements the clk API but not clk_round_rate() (None of the other clk_*
>>>>>>> symbols are undefined).
>>>>>>>
>>>>>>> Jonathan on which platform are you testing this?
>>>>>> arm specifically pxa27x eabi
>>>>>>>
>>>>>
>>>>> No clk_round_rate() in arch/arm/mach-pxa/clock.c
>>>> Hmm.. Indeed.  So what does one do if one wants to use those functions?
>>>
>>> I think the pxa implementation of the clk API is just broken in this regard.
>>> The clk API requires you to implement all the functions declared in
>>> include/linux/clk.h
>> Not according to the header it doesn't.
>>
>> /*
>>  * The remaining APIs are optional for machine class support.
>>  */
>>
>> Just above clk_round_rate
>>
>> Odd situation admittedly but the header certainly implies that these do
>> not 'need' to be implemented.
> 
> It should at least be stubbed out, otherwise this doesn't make too much sense.
> 
> - Lars
> 
So as not to step on any toes I've raised the issue on lkml and the arm kernel
list.

My personal feeling is that architectures should just implement a stub returning
an error code to indicate it isn't available but we will see what
others think.

Jonathan