2004-01-27 22:08:58

by Krzysztof Halasa

[permalink] [raw]

2004-01-28 11:48:29

by Marcelo Tosatti

[permalink] [raw]
Subject: Re: [TRIVIAL PATCH] 2.4.25pre7 warning fix



On Tue, 27 Jan 2004, Krzysztof Halasa wrote:

> Hi,
>
> The attached patch fixes the following warning msg:
>
> time.c:435: warning: `do_gettimeoffset_cyclone' defined but not used
>
> There is no need to define functions which do just { return 0; } and
> which aren't called by anything.
>
> (In case CONFIG_X86_SUMMIT is defined, there is another (real)
> do_gettimeoffset_cyclone() function, and it is referenced - but
> it's simply not related to this empty function).

Applied, thanks.

Btw, why do we need cyclone_setup() for !CONFIG_X86_SUMMIT ?

/* No-cyclone stubs */
#ifndef CONFIG_X86_SUMMIT
int __init cyclone_setup(char *str)
{
printk(KERN_ERR "cyclone: Kernel not compiled with
CONFIG_X86_SUMMIT, cannot use the cyclone-timer.\n");
return 1;
}


2004-01-28 22:24:09

by Krzysztof Halasa

[permalink] [raw]
Subject: Re: [TRIVIAL PATCH] 2.4.25pre7 warning fix

Marcelo Tosatti <[email protected]> writes:

> Btw, why do we need cyclone_setup() for !CONFIG_X86_SUMMIT ?
>
> /* No-cyclone stubs */
> #ifndef CONFIG_X86_SUMMIT
> int __init cyclone_setup(char *str)
> {
> printk(KERN_ERR "cyclone: Kernel not compiled with
> CONFIG_X86_SUMMIT, cannot use the cyclone-timer.\n");
> return 1;
> }

No idea. All the stubs seem strange to me. I will make another patch
to address that.
--
Krzysztof Halasa, B*FH

2004-01-29 16:42:20

by Krzysztof Halasa

[permalink] [raw]
Subject: Re: [TRIVIAL PATCH] 2.4.25pre7 warning fix

Marcelo Tosatti <[email protected]> writes:

> Btw, why do we need cyclone_setup() for !CONFIG_X86_SUMMIT ?
>
> /* No-cyclone stubs */
> #ifndef CONFIG_X86_SUMMIT
> int __init cyclone_setup(char *str)
> {
> printk(KERN_ERR "cyclone: Kernel not compiled with
> CONFIG_X86_SUMMIT, cannot use the cyclone-timer.\n");
> return 1;
> }

After having a closer look at it I think we should:

1. if CONFIG_X86_TSC is set:
- make calibrate_tsc() failure a fatal error
- assume use_tsc = 1 and x86_udelay_tsc = 1 and optimize them out
with preprocessor

2. if CONFIG_X86_SUMMIT is _not_ set:
- assume use_cyclone = 0 and optimize it out as well.
- cyclone_setup() etc should go out.

3. I would rename CONFIG_X86_TSC to something like CONFIG_X86_TSC_FORCE
- the current name is misleading. It wouldn't affect .config.

This is all 2.4-only, as 2.6 is a little different here.

Comments?
--
Krzysztof Halasa, B*FH

2004-01-29 19:23:14

by john stultz

[permalink] [raw]
Subject: Re: [TRIVIAL PATCH] 2.4.25pre7 warning fix

On Thu, 2004-01-29 at 08:38, Krzysztof Halasa wrote:
> Marcelo Tosatti <[email protected]> writes:
>
> > Btw, why do we need cyclone_setup() for !CONFIG_X86_SUMMIT ?
> >
> > /* No-cyclone stubs */
> > #ifndef CONFIG_X86_SUMMIT
> > int __init cyclone_setup(char *str)
> > {
> > printk(KERN_ERR "cyclone: Kernel not compiled with
> > CONFIG_X86_SUMMIT, cannot use the cyclone-timer.\n");
> > return 1;
> > }

This is needed because cyclone_setup() is called by
detect_clustered_apic(), which may or may not be done on a kernel w/
CONFIG_X86_SUMMIT enabled.

>
> After having a closer look at it I think we should:
>
> 1. if CONFIG_X86_TSC is set:
> - make calibrate_tsc() failure a fatal error
> - assume use_tsc = 1 and x86_udelay_tsc = 1 and optimize them out
> with preprocessor

Sounds fair.


>
> 2. if CONFIG_X86_SUMMIT is _not_ set:
> - assume use_cyclone = 0 and optimize it out as well.

We already do this.

> - cyclone_setup() etc should go out.

cyclone_setup() is still needed.

> 3. I would rename CONFIG_X86_TSC to something like CONFIG_X86_TSC_FORCE
> - the current name is misleading. It wouldn't affect .config.

Agreed.


thanks
-john