Patchwork [3/3] ARM: AM335x: Fix warning in timer.c

login
register
mail settings
Submitter Hunter, Jon
Date Nov. 28, 2012, 2:15 a.m.
Message ID <1354068915-3378-4-git-send-email-jon-hunter@ti.com>
Download mbox | patch
Permalink /patch/202354/
State New
Headers show

Comments

Hunter, Jon - Nov. 28, 2012, 2:15 a.m.
When compiling the kernel with configuration options ...

 # CONFIG_ARCH_OMAP2 is not set
 # CONFIG_ARCH_OMAP3 is not set
 # CONFIG_ARCH_OMAP4 is not set
 # CONFIG_SOC_OMAP5 is not set
 CONFIG_SOC_AM33XX=y

 ... the following build warning is seen.

  CC      arch/arm/mach-omap2/timer.o
  arch/arm/mach-omap2/timer.c:395:19: warning: ‘omap2_sync32k_clocksource_init’
  	defined but not used [-Wunused-function]

This issue was introduced by commit 6f80b3b (ARM: OMAP2+: timer: remove
CONFIG_OMAP_32K_TIMER) where the omap2_sync32k_clocksource_init() is no
longer referenced by the timer initialisation function for the AM335x
device as it has no 32k-sync timer.

Fix this by only including the omap2_sync32k_clocksource_init() function
if either OMAP2, OMAP3, OMAP4 or OMAP5 devices are enabled.

Cc: Igor Grinberg <grinberg@compulab.co.il>

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 arch/arm/mach-omap2/timer.c |    3 +++
 1 file changed, 3 insertions(+)
Santosh Shilimkar - Nov. 28, 2012, 6:28 a.m.
On Wednesday 28 November 2012 07:45 AM, Jon Hunter wrote:
> When compiling the kernel with configuration options ...
>
>   # CONFIG_ARCH_OMAP2 is not set
>   # CONFIG_ARCH_OMAP3 is not set
>   # CONFIG_ARCH_OMAP4 is not set
>   # CONFIG_SOC_OMAP5 is not set
>   CONFIG_SOC_AM33XX=y
>
>   ... the following build warning is seen.
>
>    CC      arch/arm/mach-omap2/timer.o
>    arch/arm/mach-omap2/timer.c:395:19: warning: ‘omap2_sync32k_clocksource_init’
>    	defined but not used [-Wunused-function]
>
> This issue was introduced by commit 6f80b3b (ARM: OMAP2+: timer: remove
> CONFIG_OMAP_32K_TIMER) where the omap2_sync32k_clocksource_init() is no
> longer referenced by the timer initialisation function for the AM335x
> device as it has no 32k-sync timer.
>
> Fix this by only including the omap2_sync32k_clocksource_init() function
> if either OMAP2, OMAP3, OMAP4 or OMAP5 devices are enabled.
>
> Cc: Igor Grinberg <grinberg@compulab.co.il>
>
> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
> ---
>   arch/arm/mach-omap2/timer.c |    3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> index eb96712..085c7e7 100644
> --- a/arch/arm/mach-omap2/timer.c
> +++ b/arch/arm/mach-omap2/timer.c
> @@ -386,6 +386,8 @@ static u32 notrace dmtimer_read_sched_clock(void)
>   	return 0;
>   }
>
> +#if defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3) || \
> +	defined(CONFIG_ARCH_OMAP4) || defined(CONFIG_SOC_OMAP5)
#ifndef CONFIG_SOC_AM33XX ?

#ifdef things are really ugly and needs constant patching and
hence something like CONFIG_HAS_32K kind of feature flags are
better. But that will undo certain part of f80b3b
(ARM: OMAP2+: timer: remove  CONFIG_OMAP_32K_TIMER).

Regards
santosh
Igor Grinberg - Nov. 28, 2012, 6:46 a.m.
On 11/28/12 08:28, Santosh Shilimkar wrote:
> On Wednesday 28 November 2012 07:45 AM, Jon Hunter wrote:
>> When compiling the kernel with configuration options ...
>>
>>   # CONFIG_ARCH_OMAP2 is not set
>>   # CONFIG_ARCH_OMAP3 is not set
>>   # CONFIG_ARCH_OMAP4 is not set
>>   # CONFIG_SOC_OMAP5 is not set
>>   CONFIG_SOC_AM33XX=y
>>
>>   ... the following build warning is seen.
>>
>>    CC      arch/arm/mach-omap2/timer.o
>>    arch/arm/mach-omap2/timer.c:395:19: warning: ‘omap2_sync32k_clocksource_init’
>>        defined but not used [-Wunused-function]
>>
>> This issue was introduced by commit 6f80b3b (ARM: OMAP2+: timer: remove
>> CONFIG_OMAP_32K_TIMER) where the omap2_sync32k_clocksource_init() is no
>> longer referenced by the timer initialisation function for the AM335x
>> device as it has no 32k-sync timer.
>>
>> Fix this by only including the omap2_sync32k_clocksource_init() function
>> if either OMAP2, OMAP3, OMAP4 or OMAP5 devices are enabled.
>>
>> Cc: Igor Grinberg <grinberg@compulab.co.il>
>>
>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>> ---
>>   arch/arm/mach-omap2/timer.c |    3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
>> index eb96712..085c7e7 100644
>> --- a/arch/arm/mach-omap2/timer.c
>> +++ b/arch/arm/mach-omap2/timer.c
>> @@ -386,6 +386,8 @@ static u32 notrace dmtimer_read_sched_clock(void)
>>       return 0;
>>   }
>>
>> +#if defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3) || \
>> +    defined(CONFIG_ARCH_OMAP4) || defined(CONFIG_SOC_OMAP5)
> #ifndef CONFIG_SOC_AM33XX ?
> 
> #ifdef things are really ugly and needs constant patching and
> hence something like CONFIG_HAS_32K kind of feature flags are
> better. But that will undo certain part of f80b3b
> (ARM: OMAP2+: timer: remove  CONFIG_OMAP_32K_TIMER).

Agreed on ugliness of ifdefs.
What about adding __maybe_unused to the function signature?
That will cover any future SoC also w/o the need to extend the ifdefs.
Santosh Shilimkar - Nov. 28, 2012, 7:20 a.m.
On Wednesday 28 November 2012 12:16 PM, Igor Grinberg wrote:
> On 11/28/12 08:28, Santosh Shilimkar wrote:
>> On Wednesday 28 November 2012 07:45 AM, Jon Hunter wrote:
>>> When compiling the kernel with configuration options ...
>>>
>>>    # CONFIG_ARCH_OMAP2 is not set
>>>    # CONFIG_ARCH_OMAP3 is not set
>>>    # CONFIG_ARCH_OMAP4 is not set
>>>    # CONFIG_SOC_OMAP5 is not set
>>>    CONFIG_SOC_AM33XX=y
>>>
>>>    ... the following build warning is seen.
>>>
>>>     CC      arch/arm/mach-omap2/timer.o
>>>     arch/arm/mach-omap2/timer.c:395:19: warning: ‘omap2_sync32k_clocksource_init’
>>>         defined but not used [-Wunused-function]
>>>
>>> This issue was introduced by commit 6f80b3b (ARM: OMAP2+: timer: remove
>>> CONFIG_OMAP_32K_TIMER) where the omap2_sync32k_clocksource_init() is no
>>> longer referenced by the timer initialisation function for the AM335x
>>> device as it has no 32k-sync timer.
>>>
>>> Fix this by only including the omap2_sync32k_clocksource_init() function
>>> if either OMAP2, OMAP3, OMAP4 or OMAP5 devices are enabled.
>>>
>>> Cc: Igor Grinberg <grinberg@compulab.co.il>
>>>
>>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>>> ---
>>>    arch/arm/mach-omap2/timer.c |    3 +++
>>>    1 file changed, 3 insertions(+)
>>>
>>> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
>>> index eb96712..085c7e7 100644
>>> --- a/arch/arm/mach-omap2/timer.c
>>> +++ b/arch/arm/mach-omap2/timer.c
>>> @@ -386,6 +386,8 @@ static u32 notrace dmtimer_read_sched_clock(void)
>>>        return 0;
>>>    }
>>>
>>> +#if defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3) || \
>>> +    defined(CONFIG_ARCH_OMAP4) || defined(CONFIG_SOC_OMAP5)
>> #ifndef CONFIG_SOC_AM33XX ?
>>
>> #ifdef things are really ugly and needs constant patching and
>> hence something like CONFIG_HAS_32K kind of feature flags are
>> better. But that will undo certain part of f80b3b
>> (ARM: OMAP2+: timer: remove  CONFIG_OMAP_32K_TIMER).
>
> Agreed on ugliness of ifdefs.
> What about adding __maybe_unused to the function signature?
> That will cover any future SoC also w/o the need to extend the ifdefs.
>
Sounds good to me.

Regards
Santosh
Hunter, Jon - Nov. 28, 2012, 4:06 p.m.
On 11/28/2012 01:20 AM, Santosh Shilimkar wrote:
> On Wednesday 28 November 2012 12:16 PM, Igor Grinberg wrote:
>> On 11/28/12 08:28, Santosh Shilimkar wrote:
>>> On Wednesday 28 November 2012 07:45 AM, Jon Hunter wrote:
>>>> When compiling the kernel with configuration options ...
>>>>
>>>>    # CONFIG_ARCH_OMAP2 is not set
>>>>    # CONFIG_ARCH_OMAP3 is not set
>>>>    # CONFIG_ARCH_OMAP4 is not set
>>>>    # CONFIG_SOC_OMAP5 is not set
>>>>    CONFIG_SOC_AM33XX=y
>>>>
>>>>    ... the following build warning is seen.
>>>>
>>>>     CC      arch/arm/mach-omap2/timer.o
>>>>     arch/arm/mach-omap2/timer.c:395:19: warning:
>>>> ‘omap2_sync32k_clocksource_init’
>>>>         defined but not used [-Wunused-function]
>>>>
>>>> This issue was introduced by commit 6f80b3b (ARM: OMAP2+: timer: remove
>>>> CONFIG_OMAP_32K_TIMER) where the omap2_sync32k_clocksource_init() is no
>>>> longer referenced by the timer initialisation function for the AM335x
>>>> device as it has no 32k-sync timer.
>>>>
>>>> Fix this by only including the omap2_sync32k_clocksource_init()
>>>> function
>>>> if either OMAP2, OMAP3, OMAP4 or OMAP5 devices are enabled.
>>>>
>>>> Cc: Igor Grinberg <grinberg@compulab.co.il>
>>>>
>>>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>>>> ---
>>>>    arch/arm/mach-omap2/timer.c |    3 +++
>>>>    1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
>>>> index eb96712..085c7e7 100644
>>>> --- a/arch/arm/mach-omap2/timer.c
>>>> +++ b/arch/arm/mach-omap2/timer.c
>>>> @@ -386,6 +386,8 @@ static u32 notrace dmtimer_read_sched_clock(void)
>>>>        return 0;
>>>>    }
>>>>
>>>> +#if defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3) || \
>>>> +    defined(CONFIG_ARCH_OMAP4) || defined(CONFIG_SOC_OMAP5)
>>> #ifndef CONFIG_SOC_AM33XX ?
>>>
>>> #ifdef things are really ugly and needs constant patching and
>>> hence something like CONFIG_HAS_32K kind of feature flags are
>>> better. But that will undo certain part of f80b3b
>>> (ARM: OMAP2+: timer: remove  CONFIG_OMAP_32K_TIMER).
>>
>> Agreed on ugliness of ifdefs.
>> What about adding __maybe_unused to the function signature?
>> That will cover any future SoC also w/o the need to extend the ifdefs.
>>
> Sounds good to me.

Yes agree on the ugliness of this. However, my thought was these would
only remain until we migrate over to device-tree and then the detection
of the a 32k source can be determine via DT.

However, I can update to use __maybe_unused for now. We just need to
remember to remove this in the future if it becomes unnecessary :-)

Cheers
Jon

Patch

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index eb96712..085c7e7 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -386,6 +386,8 @@  static u32 notrace dmtimer_read_sched_clock(void)
 	return 0;
 }
 
+#if defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3) || \
+	defined(CONFIG_ARCH_OMAP4) || defined(CONFIG_SOC_OMAP5)
 static struct of_device_id omap_counter_match[] __initdata = {
 	{ .compatible = "ti,omap-counter32k", },
 	{ }
@@ -451,6 +453,7 @@  static int __init omap2_sync32k_clocksource_init(void)
 
 	return ret;
 }
+#endif
 
 static void __init omap2_gptimer_clocksource_init(int gptimer_id,
 						const char *fck_source)