Re: [PATCH] check-dbam1-if-there-is-only-one-DIMM-on-dbam0 in unganged mode

From: wan wei
Date: Thu Aug 06 2009 - 10:58:28 EST


Well, It's just the same

On Thu, Aug 6, 2009 at 10:42 PM, Borislav Petkov<borislav.petkov@xxxxxxx> wrote:
> On Thu, Aug 06, 2009 at 06:44:34PM +0800, wan wei wrote:
>>
>>     When I populated just one DIMM for each of the two channels  of
>> the cpu node in unganged mode,   there was only one channel found by
>> edac.
>>     In counting the channels, if there is only one DIMM found by
>> dbam0,  i think dbam1 should be checked for the other one  also.
>>     Below is the patch
>>
>> Signed-off-by: Wan Wei<wanwei@xxxxxxxxxxxxxx>
>>
>> ---
>>  drivers/edac/amd64_edac.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
>> index e2a10bc..afd210a 100644
>> --- a/drivers/edac/amd64_edac.c
>> +++ b/drivers/edac/amd64_edac.c
>> @@ -1248,7 +1248,7 @@ static int f10_early_channel_count(struct amd64_pvt *pvt)
>>         /* If more than 2 DIMMs are present, then we have 2 channels */
>>         if (channels > 2)
>>                 channels = 2;
>> -       else if (channels == 0) {
>> +       else if (channels <= 1) {
>
> This is correct, we're missing the one DIMM per DCT case. Can you try
> the patch below instead - it should be equivalent to yours while making
> the code a bit more understandable.
>
> Thanks.
>
> --
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index e2a10bc..71c215b 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -1245,11 +1245,8 @@ static int f10_early_channel_count(struct amd64_pvt *pvt)
>        if (DBAM_DIMM(3, dbam) > 0)
>                channels++;
>
> -       /* If more than 2 DIMMs are present, then we have 2 channels */
> -       if (channels > 2)
> -               channels = 2;
> -       else if (channels == 0) {
> -               /* No DIMMs on DCT0, so look at DCT1 */
> +       /* check whether DCT1 has also a channel enabled */
> +       if (channels < 2) {
>                err = pci_read_config_dword(pvt->dram_f2_ctl, DBAM1, &dbam);
>                if (err)
>                        goto err_reg;
> @@ -1262,11 +1259,11 @@ static int f10_early_channel_count(struct amd64_pvt *pvt)
>                        channels++;
>                if (DBAM_DIMM(3, dbam) > 0)
>                        channels++;
> -
> -               if (channels > 2)
> -                       channels = 2;
>        }
>
> +       if (channels > 2)
> +               channels = 2;
> +
>        /* If we found ALL 0 values, then assume just ONE DIMM-ONE Channel */
>        if (channels == 0)
>                channels = 1;
>
> --
> Regards/Gruss,
> Boris.
>
> Operating | Advanced Micro Devices GmbH
>  System  | Karl-Hammerschmidt-Str. 34, 85609 Dornach b. München, Germany
>  Research | Geschäftsführer: Thomas M. McCoy, Giuliano Meroni
>  Center  | Sitz: Dornach, Gemeinde Aschheim, Landkreis München
>  (OSRC)  | Registergericht München, HRB Nr. 43632
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/