[coreboot] [PATCH] AMD revF resume support
Rudolf Marek
r.marek at assembler.cz
Mon Apr 13 20:34:49 CEST 2009
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hello,
>> +++ src/northbridge/amd/amdk8/exit_from_self.c
> ...
>> + if (dcl & DCL_DimmEccEn) {
>> + uint32_t mnc;
>
> make u32
OK this was copied perhaps.
>
> ...
>> + ".align 64\n\t"
>> + "out %%al, (%%dx) \n\t"
>> + "andb %1, %%al\n\t"
>> + "out %%al, (%%dx)\n\t"
>
> The controller might allow this but "out %%eax" would be more correct
> with the more recent pci specs.
Ok I will fix that
>
> ...
>> + if (loops >= TIMEOUT_LOOPS) {
>> + printk_debug(" failed\n");
> You might want to know which controller is was on when it failed (or passes).
OK.
>
> ...
>> + for (i = 0; i < controllers; i++) {
>> +
>> + dqs_restore_MC_NVRAM((ctrl + i)->f2);
>> +
>> + sysinfo->mem_trained[i] = 1; // mem was trained
>> +
> I assume that somewhere in the path there is a nvram valid check?
Well frankly I just did what DQS is doing to be code compatible with that.
>
>> + if (!sysinfo->ctrl_present[i])
>> + continue;
>> +
>> + /* Skip everything if I don't have any memory on this controller */
>> + if (sysinfo->meminfo[i].dimm_mask == 0x00)
>> + continue;
>
> I don't think that the two "continues" do anything here at the end.
> Should they be above the nvram and mem_trained code?
Whoops it should be down there fixed.
>
>> +++ src/northbridge/amd/amdk8/raminit_f.c
>> +#include "exit_from_self.c"
> Just add the function here since it is always included.
>
>> + int s = acpi_is_wakeup_early();
> better variable name?
Yes fixed.
>
>
>> +++ src/northbridge/amd/amdk8/raminit_f_dqs.c
>> +#ifdef S3_NVRAM_EARLY
>> +int s3_save_nvram_early(u32 dword, int size, int nvram_pos);
>> +int s3_load_nvram_early(int size, u32 *old_dword, int nvram_pos);
>> +#else
>> +int s3_save_nvram_early(u32 dword, int size, int nvram_pos) {
>> +}
>>
>> -#if MEM_TRAIN_SEQ == 0
>> +int s3_load_nvram_early(int size, u32 *old_dword, int nvram_pos) {
>> +}
>> +#endif
>
> It would be better to stub this in the file that has
> s3_save_nvram_early and add the prototype a .h.
Hmm v2 CAR is such mess. I would prefer to have it like this at least for now.
>
>> +static int load_index_to_pos(unsigned int dev, int size, int index, int nvram_pos) {
>> +
>> + u32 old_dword = pci_read_config32_index_wait(dev, 0x98, index);
>> + nvram_pos = s3_load_nvram_early(size, &old_dword, nvram_pos);
>
> What happens when this is s3_load_nvram_early doesn't do anything?
I will burn in hell ;) I will put die in this funcs as it is used just when
doing resume.
>
>> +static int dqs_load_MC_NVRAM_ch(unsigned int dev, int ch, int pos) {
> Does this play nice with the mainboard nvram map?
It is an extra memory in mine case. not a cmos one. If one needs to use CMOS
then it must play well.
>
>> +#if HAVE_ACPI_RESUME
>> + dword = 0x21132113;
> Can you comment on this setting?
Hmm I just added the response to S3 according the BKDG. You suspect anything is
wrong?
Committed revision 4102
Thanks,
Rudolf
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iEYEARECAAYFAknjhckACgkQ3J9wPJqZRNX67wCfai8sJM/OlpCbPCNSAqwdF+lz
r3UAnRgBjHdvsrulbB7sp3zfyQplHy/5
=2WML
-----END PGP SIGNATURE-----
More information about the coreboot
mailing list