diff mbox

2.6.31-rc1: sparc cannot boot

Message ID 20090618.014459.182446746.davem@davemloft.net
State Accepted
Delegated to: David Miller
Headers show

Commit Message

David Miller June 18, 2009, 8:44 a.m. UTC
From: Alexander Beregalov <a.beregalov@gmail.com>
Date: Wed, 17 Jun 2009 19:00:18 +0400

> Config is attached.

Aha, the key is non-SMP build.

> b696fdc259f0d94348a9327bed352fac44d4883d is first bad commit
> sparc64: Defer cpu_data() setup until end of per-cpu data initialization.
> 
> Does it make sense to you?

I see the problem, please try this patch:

sparc64: Fix UP bootup regression.

Commit b696fdc259f0d94348a9327bed352fac44d4883d ("sparc64: Defer
cpu_data() setup until end of per-cpu data initialization.") broke
bootup for UP builds because the cpu_data() initialization only
occurs in setup_per_cpu_areas() which is never compiled in nor
called in UP builds.

Fix this up by calling the setups directly from init_64.c when
non-SMP.

Reported-by: Alexander Beregalov <a.beregalov@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 arch/sparc/mm/init_64.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

Comments

Alexander Beregalov June 18, 2009, 10:47 a.m. UTC | #1
2009/6/18 David Miller <davem@davemloft.net>:
> From: Alexander Beregalov <a.beregalov@gmail.com>
> Date: Wed, 17 Jun 2009 19:00:18 +0400
>
>> Config is attached.
>
> Aha, the key is non-SMP build.
>
>> b696fdc259f0d94348a9327bed352fac44d4883d is first bad commit
>> sparc64: Defer cpu_data() setup until end of per-cpu data initialization.
>>
>> Does it make sense to you?
>
> I see the problem, please try this patch:
>
> sparc64: Fix UP bootup regression.

Hm, no, still the same problem.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller June 18, 2009, 11:02 a.m. UTC | #2
From: Alexander Beregalov <a.beregalov@gmail.com>
Date: Thu, 18 Jun 2009 14:47:57 +0400

> 2009/6/18 David Miller <davem@davemloft.net>:
>> From: Alexander Beregalov <a.beregalov@gmail.com>
>> Date: Wed, 17 Jun 2009 19:00:18 +0400
>>
>>> Config is attached.
>>
>> Aha, the key is non-SMP build.
>>
>>> b696fdc259f0d94348a9327bed352fac44d4883d is first bad commit
>>> sparc64: Defer cpu_data() setup until end of per-cpu data initialization.
>>>
>>> Does it make sense to you?
>>
>> I see the problem, please try this patch:
>>
>> sparc64: Fix UP bootup regression.
> 
> Hm, no, still the same problem.

The only thing that explains your bug is that cpu_data()->clock_tick
is zero.

And that would be zero if you have SMP disabled because of the bug
I am fixing.

of_fill_in_cpu_data() should find the right value and fill it in after
my patch from paging_init(), which is long before time_init() which is
where your problem triggers.

See what value sparc64_init_timers() ends up using for "freq".

Thanks!
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Beregalov June 18, 2009, 11:53 a.m. UTC | #3
2009/6/18 David Miller <davem@davemloft.net>:
> From: Alexander Beregalov <a.beregalov@gmail.com>
> Date: Thu, 18 Jun 2009 14:47:57 +0400
>
>> 2009/6/18 David Miller <davem@davemloft.net>:
>>> From: Alexander Beregalov <a.beregalov@gmail.com>
>>> Date: Wed, 17 Jun 2009 19:00:18 +0400
>>>
>>>> Config is attached.
>>>
>>> Aha, the key is non-SMP build.
>>>
>>>> b696fdc259f0d94348a9327bed352fac44d4883d is first bad commit
>>>> sparc64: Defer cpu_data() setup until end of per-cpu data initialization.
>>>>
>>>> Does it make sense to you?
>>>
>>> I see the problem, please try this patch:
>>>
>>> sparc64: Fix UP bootup regression.
>>
>> Hm, no, still the same problem.
>
> The only thing that explains your bug is that cpu_data()->clock_tick
> is zero.
>
> And that would be zero if you have SMP disabled because of the bug
> I am fixing.
>
> of_fill_in_cpu_data() should find the right value and fill it in after
> my patch from paging_init(), which is long before time_init() which is
> where your problem triggers.
>
> See what value sparc64_init_timers() ends up using for "freq".
It returns freq=0.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller June 18, 2009, 11:58 a.m. UTC | #4
From: Alexander Beregalov <a.beregalov@gmail.com>
Date: Thu, 18 Jun 2009 15:53:40 +0400

>> See what value sparc64_init_timers() ends up using for "freq".
> It returns freq=0.

Please, dig deeper :-)

Where is it getting it from?  cpu_data()->clock_tick?

If so, see if of_fill_in_cpu_data() is even running, and if so
why it isn't storing a non-zero value there.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Beregalov June 18, 2009, 3:14 p.m. UTC | #5
2009/6/18 David Miller <davem@davemloft.net>:
> From: Alexander Beregalov <a.beregalov@gmail.com>
> Date: Thu, 18 Jun 2009 15:53:40 +0400
>
>>> See what value sparc64_init_timers() ends up using for "freq".
>> It returns freq=0.
>
> Please, dig deeper :-)
>
> Where is it getting it from?  cpu_data()->clock_tick?
Yes, exactly.
freq = local_cpu_data().clock_tick;

>
> If so, see if of_fill_in_cpu_data() is even running, and if so
> why it isn't storing a non-zero value there.

Oops. Digging deeper I found that I incorrectly applied your patch.
I manually edited init_64.c and wrote #ifdef instead of #ifndef.
Sorry :)

Your patch is really working.
Thanks!

fill_in_one_cpu():
<...>
        cpu_data(cpuid).clock_tick =
                of_getintprop_default(dp, "clock-frequency", 0);
printk("cpuid: %u; clock_tick: %lu\n", cpuid,  cpu_data(cpuid).clock_tick);

prints
cpuid: 0; clock_tick: 440000000
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller June 18, 2009, 6:50 p.m. UTC | #6
From: Alexander Beregalov <a.beregalov@gmail.com>
Date: Thu, 18 Jun 2009 19:14:48 +0400

> Oops. Digging deeper I found that I incorrectly applied your patch.
> I manually edited init_64.c and wrote #ifdef instead of #ifndef.
> Sorry :)
> 
> Your patch is really working.

I was hoping you would say that, thanks for testing! :-)

--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c
index ca92e2f..ed6be6b 100644
--- a/arch/sparc/mm/init_64.c
+++ b/arch/sparc/mm/init_64.c
@@ -1796,10 +1796,16 @@  void __init paging_init(void)
 
 	prom_build_devicetree();
 	of_populate_present_mask();
+#ifndef CONFIG_SMP
+	of_fill_in_cpu_data();
+#endif
 
 	if (tlb_type == hypervisor) {
 		sun4v_mdesc_init();
 		mdesc_populate_present_mask(cpu_all_mask);
+#ifndef CONFIG_SMP
+		mdesc_fill_in_cpu_data(cpu_all_mask);
+#endif
 	}
 
 	/* Once the OF device tree and MDESC have been setup, we know