Skip to contentSkip to conversation

timekeeping: Fix up read_persistent_clock() breakage on sh.

PATCH Applied Paul Mundt 2 participants 5 messages Aug 24, 2009 – Aug 25, 2009 SuperH

lore
Share & tools
lore

Paul Mundt

author
#0 lore raw

PATCH 1/1 timekeeping: Fix up read_persistent_clock() breakage on sh.

The recent commit "timekeeping: Increase granularity of read_persistent_clock()" introduced read_persistent_clock() rework which inadvertently broke the sh conversion:

arch/sh/kernel/time.c:45: error: passing argument 1 of 'rtc_sh_get_time' from incompatible pointer type distcc[13470] ERROR: compile arch/sh/kernel/time.c on sprygo/32 failed
make[2]: *** [arch/sh/kernel/time.o] Error 1

This trivial fix gets it working again.

Cc Martin Schwidefsky <[email protected]>
Cc Ingo Molnar <[email protected]>

---

 arch/sh/kernel/time.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/sh/kernel/time.c +1 -1
index 3f4706a..0e0e858 100644
@@ -41,7 +41,7 @@ int (*rtc_sh_set_time)(const time_t) = null_rtc_set_time;
 #ifdef CONFIG_GENERIC_CMOS_UPDATE
 void read_persistent_clock(struct timespec *ts)
 {
-	rtc_sh_get_time(&ts);
+	rtc_sh_get_time(ts);
 }
 
 int update_persistent_clock(struct timespec now)
Applied
#1 lore raw
Quoted 10 lines · Paul Mundt

The recent commit "timekeeping: Increase granularity of read_persistent_clock()" introduced read_persistent_clock() rework which inadvertently broke the sh conversion:

arch/sh/kernel/time.c:45: error: passing argument 1 of 'rtc_sh_get_time' from incompatible pointer type distcc[13470] ERROR: compile arch/sh/kernel/time.c on sprygo/32 failed
make[2]: *** [arch/sh/kernel/time.o] Error 1

This trivial fix gets it working again.

Quoted 8 lines
Cc Martin Schwidefsky <[email protected]>
Cc Ingo Molnar <[email protected]>

---

 arch/sh/kernel/time.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

applied, thanks Paul!

I'm wondering, why did my cross-build of the SH defconfig in the timer tree not catch this? All i get (and got) are these warnings:

/home/mingo/tip/arch/sh/kernel/cpu/clock-cpg.c: In function 'cpg_clk_init':
/home/mingo/tip/arch/sh/kernel/cpu/clock-cpg.c:242: warning: 'arch_init_clk_ops' is deprecated (declared at /home/mingo/tip/arch/sh/include/asm/clock.h:58) /home/mingo/tip/arch/sh/kernel/cpu/clock-cpg.c: In function 'arch_clk_init':
/home/mingo/tip/arch/sh/kernel/cpu/clock-cpg.c:256: warning: 'cpg_clk_init' is deprecated (declared at /home/mingo/tip/arch/sh/kernel/cpu/clock-cpg.c:237)

and time.o built without errors or warnings:

CC arch/sh/kernel/time.o

Ingo

author
#2 lore raw
Quoted 27 lines · Ingo Molnar
Quoted 10 lines · Paul Mundt

The recent commit "timekeeping: Increase granularity of read_persistent_clock()" introduced read_persistent_clock() rework which inadvertently broke the sh conversion:

arch/sh/kernel/time.c:45: error: passing argument 1 of 'rtc_sh_get_time' from incompatible pointer type distcc[13470] ERROR: compile arch/sh/kernel/time.c on sprygo/32 failed
make[2]: *** [arch/sh/kernel/time.o] Error 1

This trivial fix gets it working again.

Quoted 8 lines
Cc Martin Schwidefsky <[email protected]>
Cc Ingo Molnar <[email protected]>

---

 arch/sh/kernel/time.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

applied, thanks Paul!

I'm wondering, why did my cross-build of the SH defconfig in the timer tree not catch this? All i get (and got) are these warnings:

Quoted 9 lines

/home/mingo/tip/arch/sh/kernel/cpu/clock-cpg.c: In function 'cpg_clk_init':
/home/mingo/tip/arch/sh/kernel/cpu/clock-cpg.c:242: warning: 'arch_init_clk_ops' is deprecated (declared at /home/mingo/tip/arch/sh/include/asm/clock.h:58) /home/mingo/tip/arch/sh/kernel/cpu/clock-cpg.c: In function 'arch_clk_init':
/home/mingo/tip/arch/sh/kernel/cpu/clock-cpg.c:256: warning: 'cpg_clk_init' is deprecated (declared at /home/mingo/tip/arch/sh/kernel/cpu/clock-cpg.c:237)

and time.o built without errors or warnings:

CC arch/sh/kernel/time.o

That would be because read_persistent_clock() is only used conditionally on SH, the defconfig itself does not make use of it, while the dreamcast and sh03 defconfigs do. SH has a lot of variation across its defconfigs, so it's pretty difficult to catch everything, this is largely what we rely on randconfigs and allmod/yes/noconfigs for, which are all part of the regular daily builds, in addition to the 50 or so other configs.

The defconfig itself is aimed at covering the common cases. Config options that people will generally have enabled, and are likely to run in to issues with. Anything else we just have to fix up incrementally and hope that daily builds/regression testing catches the rest.

ARM suffers from the same problem in that it's really impossible to get decent coverage in a single config.

#3 lore raw
Quoted 45 lines · Paul Mundt
Quoted 27 lines · Ingo Molnar
Quoted 10 lines · Paul Mundt

The recent commit "timekeeping: Increase granularity of read_persistent_clock()" introduced read_persistent_clock() rework which inadvertently broke the sh conversion:

arch/sh/kernel/time.c:45: error: passing argument 1 of 'rtc_sh_get_time' from incompatible pointer type distcc[13470] ERROR: compile arch/sh/kernel/time.c on sprygo/32 failed
make[2]: *** [arch/sh/kernel/time.o] Error 1

This trivial fix gets it working again.

Quoted 8 lines
Cc Martin Schwidefsky <[email protected]>
Cc Ingo Molnar <[email protected]>

---

 arch/sh/kernel/time.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

applied, thanks Paul!

I'm wondering, why did my cross-build of the SH defconfig in the timer tree not catch this? All i get (and got) are these warnings:

Quoted 8 lines

/home/mingo/tip/arch/sh/kernel/cpu/clock-cpg.c: In function 'cpg_clk_init':
/home/mingo/tip/arch/sh/kernel/cpu/clock-cpg.c:242: warning: 'arch_init_clk_ops' is deprecated (declared at /home/mingo/tip/arch/sh/include/asm/clock.h:58) /home/mingo/tip/arch/sh/kernel/cpu/clock-cpg.c: In function 'arch_clk_init':
/home/mingo/tip/arch/sh/kernel/cpu/clock-cpg.c:256: warning: 'cpg_clk_init' is deprecated (declared at /home/mingo/tip/arch/sh/kernel/cpu/clock-cpg.c:237)

and time.o built without errors or warnings:

CC arch/sh/kernel/time.o

That would be because read_persistent_clock() is only used conditionally on SH, the defconfig itself does not make use of it, while the dreamcast and sh03 defconfigs do. SH has a lot of variation across its defconfigs, so it's pretty difficult to catch everything, this is largely what we rely on randconfigs and allmod/yes/noconfigs for, which are all part of the regular daily builds, in addition to the 50 or so other configs.

Quoted 8 lines

The defconfig itself is aimed at covering the common cases. Config options that people will generally have enabled, and are likely to run in to issues with. Anything else we just have to fix up incrementally and hope that daily builds/regression testing catches the rest.

ARM suffers from the same problem in that it's really impossible to get decent coverage in a single config.

Yeah - x86 is really similar as well.

So the general policy we typically follow is to expect this from developers:

- test the patch on their own favorite config[s] they actually use, on the architecture[s] they care about personally.

- do and send clean patches, use git grep, etc.

And then we couple that with fast patch and fix propagation pathways into integration trees, which then do stronger testing.

It would be a mistake to expect all kernel developers that submit patches to build all the 300+ defconfigs that are in the upstream kernel currently (!) - in fact it's a waste of resources to expect them to 'always' build configs they dont actually use. (They dont have the hardware to do such testing and they dont have the time and interest to do such testing.)

And, since Linux changes/adds/removes ~1 million lines of code in every development cycle, we should expect breakages and should handle them quickly - not put pressure on developers to find bugs they have no chance to trigger realistically.

Lets face it, the 22 Linux architectures we have are an exponential testing nightmare. There's not even a common set of cross-compilers from-x86-to-arch for people to use, one has to build it from source one by one, and (here i speak from experience) GCC rarely builds cross-compilers out of box without some kind of build/bootstrap trouble. There's architectures where neither gcc-trunk, 4.4, 4.3, 4.2 nor 4.1 builds a cross-compiler correctly - only some old 3.x compiler works.

(And even if we had correctly packaged, readily available cross-compilers for x86, it would still slow down the testing of patches 20-fold.)

Ingo

#4 lore raw

[tip:timers/core] timekeeping: Fix up read_persistent_clock() breakage on sh

In reply to Paul Mundt
Commit-ID:  0ceb4c3e3f1ccaf121851e33c3ea269b8ad0f219
Gitweb:     http://git.kernel.org/tip/0ceb4c3e3f1ccaf121851e33c3ea269b8ad0f219
Author:     Paul Mundt <[email protected]>
AuthorDate: Tue, 25 Aug 2009 07:32:39 +0900
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 25 Aug 2009 08:46:16 +0200

timekeeping: Fix up read_persistent_clock() breakage on sh

The recent commit "timekeeping: Increase granularity of read_persistent_clock()" introduced read_persistent_clock() rework which inadvertently broke the sh conversion:

arch/sh/kernel/time.c:45: error: passing argument 1 of 'rtc_sh_get_time' from incompatible pointer type distcc[13470] ERROR: compile arch/sh/kernel/time.c on sprygo/32 failed
make[2]: *** [arch/sh/kernel/time.o] Error 1

This trivial fix gets it working again.

Cc Martin Schwidefsky <[email protected]>

LKML-Reference: <[email protected]>

---
 arch/sh/kernel/time.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/sh/kernel/time.c +1 -1
index 3f4706a..0e0e858 100644
@@ -41,7 +41,7 @@ int (*rtc_sh_set_time)(const time_t) = null_rtc_set_time;
 #ifdef CONFIG_GENERIC_CMOS_UPDATE
 void read_persistent_clock(struct timespec *ts)
 {
-	rtc_sh_get_time(&ts);
+	rtc_sh_get_time(ts);
 }
 
 int update_persistent_clock(struct timespec now)