Re: rtc-cmos.c: Build fix

From: Carlos R. Mafra
Date: Tue May 06 2008 - 14:53:33 EST


> since you seem to be interested in HPET topics, what do you think about
> the patch below from akpm? It had a build failure with this config:
>
> http://redhat.com/~mingo/misc/config-Sun_May__4_09_41_21_CEST_2008.bad

I think I've found the problem. After fixing the rtc-cmos.o build your
.config produced yet another failure later on:

LD .tmp_vmlinux1
drivers/built-in.o: In function `v4l2_i2c_drv_attach_legacy':
tuner-core.c:(.text+0xc5a31): undefined reference to `v4l2_i2c_attach'
drivers/built-in.o: In function `tuner_command':
tuner-core.c:(.text+0xc6dc2): undefined reference to `v4l_printk_ioctl'
make: *** [.tmp_vmlinux1] Error 1

But this one I left untouched for the moment.

> ------------->
> From: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
>
> Should already be available via the hpet.h inclusion.

With the inclusion of hpet.h we don't have the "do-nothing stubs" for
the case !HPET_EMULATE_RTC as they are defined inside rtc.c
and rtc-cmos.c. However hpet_rtc_interrupt() is missing in those
stubs because it was removed with akpm's patch. So I added it.

My explanation for the build failure is as follows.

Your .config did not have HPET_EMULATE_RTC enabled but the code
in rtc-cmos.c wanted to use hpet_rtc_interrupt() anyways. But
looking in arch/x86/kernel/hpet.c this function is inside a

#ifdef CONFIG_HPET_EMULATE_RTC <-- line 465
hpet_rtc_interrupt() <--- line 679
#endif <--- line 716

so it should be used if and only if HPET_EMULATE_RTC is defined.

And the code in question which makes the build fail is inside a
if(is_hpet_enabled()) {
...
rtc_cmos_int_handler = hpet_rtc_interrupt;
...
}
and this would never be executed because with !HPET_EMULATE_RTC (as
in your .config) is_hpet_enabled() is defined to return 0.

So I think Andrew's patch should be the one below (I've folded
my correction into his patch, so I am Cc:ing him also), where my
modification is the adition inside the #ifndef CONFIG_HPET_EMULATE_RTC
(which I took from rtc.c)

> Could go further, by defining the do-nothing stub in hpet.h as well, perhaps.

I think one could do that too as a cleanup (to remove the do-nothing stubs
from rtc.c and rtc-cmos.c), but I am afraid to make a mistake which would
cause other build failures afterwards. So now I just wanted to understand
and fix the issue you suggested to me (for which I thank you).

Furthermore, the modification in rtc.c is ok because it was used only
if CONFIG_HPET_EMULATE_RTC is defined, but that can only happen if
CONFIG_HPET_TIMER is also defined (due to 'depends on HPET_TIMER' in
the Kconfig), in which case the inclusion of hpet.h is effective.

I hope this is ok for now.