On Oct 13, 2010, at 10:58 AM, Scott Wood wrote:
> On Wed, 13 Oct 2010 08:17:11 -0500
> Kumar Gala <ga...@kernel.crashing.org> wrote:
>
>> The new e5500 core is similar to the e500mc core but adds 64-bit
>> support. We support running it in 32-bit mode as it is identical to the
>> e500mc.
>>
>> Signed-off-by: Kumar Gala <ga...@kernel.crashing.org>
>> ---
>> * clean up kconfig further to reduce use of E500MC
>
> Looks better, just a few nits:
>
>> -#ifdef CONFIG_E500
>> +#if defined(CONFIG_E500) || defined(CONFIG_PPC_BOOK3E_64)
>> /* All e500 */
>> #define MCSR_MCP 0x80000000UL /* Machine Check Input Pin */
>> #define MCSR_ICPERR 0x40000000UL /* I-Cache Parity Error */
>
> Is this really supposed to be here for all 64-bit book3e? Likewise in
> the C code.
This is a question I need to ask BenH about. If we intend or desire to have a
single kernel image that works on all 64-bit book3e parts or not.
>
>> +obj-$(CONFIG_PPC_BOOK3E_64) += cpu_setup_fsl_booke.o
>
> CONFIG_PPC_FSL_BOOK3E?
will fix.
>> @@ -66,6 +66,10 @@ extern void __restore_cpu_ppc970(void);
>> extern void __setup_cpu_power7(unsigned long offset, struct cpu_spec* spec);
>> extern void __restore_cpu_power7(void);
>> #endif /* CONFIG_PPC64 */
>> +#if defined(CONFIG_PPC_BOOK3E_64) || defined(CONFIG_E500)
>> +extern void __setup_cpu_e5500(unsigned long offset, struct cpu_spec* spec);
>> +extern void __restore_cpu_e5500(void);
>> +#endif /* CONFIG_PPC_BOOK3E_64 || CONFIG_E500 */
>
> CONFIG_E500 should be sufficient. Not sure why these need to be
> ifdeffed at all, though it seems to be existing practice here.
again, about if we want a generic ppc64e build or not, but I'm now thinking
that would end up setting CONFIG_E500 anyways.
>> /* This table only contains "desktop" CPUs, it need to be filled with
>> embedded
>> * ones as well...
>> @@ -1891,7 +1895,28 @@ static struct cpu_spec __initdata cpu_specs[] = {
>> .platform = "ppc5554",
>> }
>> #endif /* CONFIG_E200 */
>> -#ifdef CONFIG_E500
>> +#endif /* CONFIG_PPC32 */
>> +#if defined(CONFIG_PPC_BOOK3E_64) || defined(CONFIG_E500)
>
> Just E500 should work.
>
>> @@ -538,6 +538,11 @@ int machine_check_e500(struct pt_regs *regs)
>>
>> return 0;
>> }
>> +
>> +int machine_check_generic(struct pt_regs *regs)
>> +{
>> + return 0;
>> +}
>
> Hmm, it seems that either the cputable entry that references this
> should not be built in if we don't support those chips, or the real
> implementation shouldn't be under an #else if we are going to support
> multiplatform coexistence with them.
- k
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
