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

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

[wan wei]

    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@dawning.com.cn>

---
 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) {
                /* No DIMMs on DCT0, so look at DCT1 */
                err = pci_read_config_dword(pvt->dram_f2_ctl, DBAM1, &dbam);
                if (err)
-- 
1.4.4.2

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

[Borislav Petkov]

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@dawning.com.cn>
> 
> ---
>  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

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

[wan wei]

Well, It's just the same

On Thu, Aug 6, 2009 at 10:42 PM, Borislav Petkov<borislav.petkov@amd.com> 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@dawning.com.cn>
>>
>> ---
>>  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
>
>

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

[Borislav Petkov]

On Thu, Aug 06, 2009 at 10:58:21PM +0800, wan wei wrote:
> Well, It's just the same

Did you test it on your machine?

-- 
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