s289{1,2,5} whitespace (3583) and questions

[Myles Watson]

I'd like to understand the v2 code for the Tyan s289{1,2,5} boards.  As part
of that I fixed some whitespace.  It was committed in 3583.

Here's a list of my questions so far (I'm trying to understand differences)

auto.c:

Why does s2895 include arch/cpu.h and amd/model_fxx/model_fxx_msr.h
Why does s2895 not define K8_HT_FREQ_1G_SUPPORT
Why does s2892 have report_bist_failure commented out

cache_as_ram_auto.c:

Why does s2892 have the soft_reset commented out, but still print ht_reset
Why does s2892 not get the return value

Options.lb:
Why is _RAMBASE different for s2895 (0x100000) than s2892 and s2891 (0x4000)
Why does s2891 use CONFIG_PCI_64BIT_PREF_MEM=1, but 2 and 5 don't

These three lines are missing from s2892:
default ENABLE_APIC_EXT_ID=0
default APIC_ID_OFFSET=0x10
default LIFT_BSP_APIC_ID=0

failover.c:
Only s2891 passes nodeid to early_mtrr_init_detected
Only s2895 doesn't call enable_lapic()

irq_tables.c:
All the #if 0 code, is it needed for documentation?

Thanks,
Myles

[Peter Stuge]

Myles Watson wrote:
> I'd like to understand the v2 code for the Tyan s289{1,2,5} boards.
> As part of that I fixed some whitespace.  It was committed in 3583.

Awesome.
> Here's a list of my questions so far (I'm trying to understand
> differences)

I don't recall if this was ever reviewed very thoroughly or just went
in in bulk like mcp55, but I do know that only Yinghai had access to
the NVIDIA chipset documentation. I hope he can provide some answers.

//Peter

[yhlu]

On Thu, Sep 18, 2008 at 8:32 AM, Myles Watson  wrote:
> I'd like to understand the v2 code for the Tyan s289{1,2,5} boards.  As part
> of that I fixed some whitespace.  It was committed in 3583.
>
> Here's a list of my questions so far (I'm trying to understand differences)
>
> auto.c:
>
> Why does s2895 include arch/cpu.h and amd/model_fxx/model_fxx_msr.h
> Why does s2895 not define K8_HT_FREQ_1G_SUPPORT

it is not tested
> Why does s2892 have report_bist_failure commented out
>
> cache_as_ram_auto.c:
>
> Why does s2892 have the soft_reset commented out, but still print ht_reset
> Why does s2892 not get the return value
>
> Options.lb:
> Why is _RAMBASE different for s2895 (0x100000) than s2892 and s2891 (0x4000)

should be ok.
> Why does s2891 use CONFIG_PCI_64BIT_PREF_MEM=1, but 2 and 5 don't

only test with that with that card need that feature.
> These three lines are missing from s2892:
> default ENABLE_APIC_EXT_ID=0
> default APIC_ID_OFFSET=0x10
> default LIFT_BSP_APIC_ID=0

should be ok, already disable with
ENABLE_APIC_EXT_ID=0
> failover.c:
> Only s2891 passes nodeid to early_mtrr_init_detected
> Only s2895 doesn't call enable_lapic()
>
> irq_tables.c:
> All the #if 0 code, is it needed for documentation?

should be corrected, so some uitil could do mapping between bus/dev to slot...

YH

[Myles Watson]

> -----Original Message-----
> From: yhlu [mailto:[e-mail]]
> Sent: Saturday, September 20, 2008 4:06 PM
> To: Myles Watson
> Cc: [e-mail]
> Subject: Re: [coreboot] s289{1,2,5} whitespace (3583) and questions
>
> On Thu, Sep 18, 2008 at 8:32 AM, Myles Watson  wrote:
> > I'd like to understand the v2 code for the Tyan s289{1,2,5} boards.  As
>
> part
> > of that I fixed some whitespace.  It was committed in 3583.
> >
> > Here's a list of my questions so far (I'm trying to understand
>
> differences)
> > auto.c:
> >
> > Why does s2895 include arch/cpu.h and amd/model_fxx/model_fxx_msr.h
> > Why does s2895 not define K8_HT_FREQ_1G_SUPPORT
>
> it is not tested
> > Why does s2892 have report_bist_failure commented out
> >
> > cache_as_ram_auto.c:
> >
> > Why does s2892 have the soft_reset commented out, but still print
>
> ht_reset
> > Why does s2892 not get the return value
> >
> > Options.lb:
> > Why is _RAMBASE different for s2895 (0x100000) than s2892 and s2891
>
> (0x4000)
>
> should be ok.
> > Why does s2891 use CONFIG_PCI_64BIT_PREF_MEM=1, but 2 and 5 don't
>
> only test with that with that card need that feature.
> > These three lines are missing from s2892:
> > default ENABLE_APIC_EXT_ID=0
> > default APIC_ID_OFFSET=0x10
> > default LIFT_BSP_APIC_ID=0
>
> should be ok, already disable with
> ENABLE_APIC_EXT_ID=0
> > failover.c:
> > Only s2891 passes nodeid to early_mtrr_init_detected
> > Only s2895 doesn't call enable_lapic()
> >
> > irq_tables.c:
> > All the #if 0 code, is it needed for documentation?
>
> should be corrected, so some uitil could do mapping between bus/dev to
> slot...

Thanks,
Myles