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

From: Borislav Petkov
Date: Thu Aug 06 2009 - 10:43:04 EST


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/