[PATCH V2 0/3] Updates to amd64_edac

[PATCH V2 0/3] Updates to amd64_edac

[Aravind Gopalakrishnan]

[V1 patchset: http://marc.info/?l=linux-kernel&m=144243636509201&w=2
Patch1 of above set was accepted already. So, not including that as
part of this V2 set.]

Patches in V2-
Patch 1: For F15hM60h, register used to program scrub rate has changed.
	 Add support in the code for the new register
Patch 2: Remove changelogs and trim comments section in amd64_edac.h
Patch 3: Add back some of the comments regarding reference documents 
	 from amd64_edac.h to Documentation/edac.txt

Changes from V1 (per Boris suggestions)-
  - Use family/model checks in __set_scrub_rate() and write appropriate
    scrub rate register there instead of re-factoring code.
  - Removed code to set dct in get_scrub_rate() as it was unnecessary
  - Removed changelogs from amd64_edac.h and add back any info about
    reference documents to Documentation/edac.txt

Aravind Gopalakrishnan (3):
  EDAC, amd64_edac: Extend scrub rate programmability feature for
    F15hM60h
  EDAC, amd64_edac: Update copyright and remove changelog
  Documentation/edac: Add reference documents section for amd64_edac

 Documentation/edac.txt    | 46 +++++++++++++++++++++++++++++++++++++
 drivers/edac/amd64_edac.c | 23 +++++++++++++++----
 drivers/edac/amd64_edac.h | 58 +++--------------------------------------------
 3 files changed, 68 insertions(+), 59 deletions(-)

-- 
2.4.0

[PATCH V2 1/3] EDAC, amd64_edac: Extend scrub rate programmability feature for F15hM60h

[Aravind Gopalakrishnan]

For F15h M60h processor, the scrub rate control register has moved
to F2 of PCI config space and is at a different offset from
earlier processors. The minimun recommended scrub rate is also different.
(Refer D18F2x1c9_dct[1:0][DramScrub] on Fam15hM60h BKDG)

Modify the set_scrub_rate() and get_scrub_rate() functions so that
they are aware of these changes.

Tested on F15hM60h, Fam15h Models 00h-0fh and Fam10h systems and
it works fine.

Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
---
 drivers/edac/amd64_edac.c | 23 +++++++++++++++++++----
 drivers/edac/amd64_edac.h |  2 ++
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 73aea40..0ae72b9 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -173,7 +173,7 @@ static inline int amd64_read_dct_pci_cfg(struct amd64_pvt *pvt, u8 dct,
  * scan the scrub rate mapping table for a close or matching bandwidth value to
  * issue. If requested is too big, then use last maximum value found.
  */
-static int __set_scrub_rate(struct pci_dev *ctl, u32 new_bw, u32 min_rate)
+static int __set_scrub_rate(struct amd64_pvt *pvt, u32 new_bw, u32 min_rate)
 {
 	u32 scrubval;
 	int i;
@@ -201,7 +201,14 @@ static int __set_scrub_rate(struct pci_dev *ctl, u32 new_bw, u32 min_rate)
 
 	scrubval = scrubrates[i].scrubval;
 
-	pci_write_bits32(ctl, SCRCTRL, scrubval, 0x001F);
+	if (pvt->fam == 0x15 && pvt->model == 0x60) {
+		f15h_select_dct(pvt, 0);
+		pci_write_bits32(pvt->F2, F15H_M60H_SCRCTRL, scrubval, 0x001F);
+		f15h_select_dct(pvt, 1);
+		pci_write_bits32(pvt->F2, F15H_M60H_SCRCTRL, scrubval, 0x001F);
+	} else {
+		pci_write_bits32(pvt->F3, SCRCTRL, scrubval, 0x001F);
+	}
 
 	if (scrubval)
 		return scrubrates[i].bandwidth;
@@ -216,12 +223,17 @@ static int set_scrub_rate(struct mem_ctl_info *mci, u32 bw)
 
 	if (pvt->fam == 0xf)
 		min_scrubrate = 0x0;
+	else if (pvt->fam == 0x15 && pvt->model == 0x60)
+		min_scrubrate = 0x6;
 
 	/* Erratum #505 */
 	if (pvt->fam == 0x15 && pvt->model < 0x10)
 		f15h_select_dct(pvt, 0);
 
-	return __set_scrub_rate(pvt->F3, bw, min_scrubrate);
+	if (pvt->fam == 0x15 && pvt->model == 0x60)
+		return __set_scrub_rate(pvt, bw, min_scrubrate);
+
+	return __set_scrub_rate(pvt, bw, min_scrubrate);
 }
 
 static int get_scrub_rate(struct mem_ctl_info *mci)
@@ -234,7 +246,10 @@ static int get_scrub_rate(struct mem_ctl_info *mci)
 	if (pvt->fam == 0x15 && pvt->model < 0x10)
 		f15h_select_dct(pvt, 0);
 
-	amd64_read_pci_cfg(pvt->F3, SCRCTRL, &scrubval);
+	if (pvt->fam == 0x15 && pvt->model == 0x60)
+		amd64_read_pci_cfg(pvt->F2, F15H_M60H_SCRCTRL, &scrubval);
+	else
+		amd64_read_pci_cfg(pvt->F3, SCRCTRL, &scrubval);
 
 	scrubval = scrubval & 0x001F;
 
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index 4bdec75..971dc12 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -255,6 +255,8 @@
 
 #define DCT_SEL_HI			0x114
 
+#define F15H_M60H_SCRCTRL		0x1C8
+
 /*
  * Function 3 - Misc Control
  */
-- 
2.4.0

[PATCH V2 2/3] EDAC, amd64_edac: Update copyright and remove changelog

[Aravind Gopalakrishnan]

Git provides us all the changelogs anyway. So trimming
the comments section here.

Updated the copyrights info while at it.

Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
---
 drivers/edac/amd64_edac.h | 56 +----------------------------------------------
 1 file changed, 1 insertion(+), 55 deletions(-)

diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index 971dc12..c0f248f 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -2,64 +2,10 @@
  * AMD64 class Memory Controller kernel module
  *
  * Copyright (c) 2009 SoftwareBitMaker.
- * Copyright (c) 2009 Advanced Micro Devices, Inc.
+ * Copyright (c) 2009-15 Advanced Micro Devices, Inc.
  *
  * This file may be distributed under the terms of the
  * GNU General Public License.
- *
- *	Originally Written by Thayne Harbaugh
- *
- *      Changes by Douglas "norsk" Thompson  <dougthompson@xmission.com>:
- *		- K8 CPU Revision D and greater support
- *
- *      Changes by Dave Peterson <dsp@llnl.gov> <dave_peterson@pobox.com>:
- *		- Module largely rewritten, with new (and hopefully correct)
- *		code for dealing with node and chip select interleaving,
- *		various code cleanup, and bug fixes
- *		- Added support for memory hoisting using DRAM hole address
- *		register
- *
- *	Changes by Douglas "norsk" Thompson <dougthompson@xmission.com>:
- *		-K8 Rev (1207) revision support added, required Revision
- *		specific mini-driver code to support Rev F as well as
- *		prior revisions
- *
- *	Changes by Douglas "norsk" Thompson <dougthompson@xmission.com>:
- *		-Family 10h revision support added. New PCI Device IDs,
- *		indicating new changes. Actual registers modified
- *		were slight, less than the Rev E to Rev F transition
- *		but changing the PCI Device ID was the proper thing to
- *		do, as it provides for almost automactic family
- *		detection. The mods to Rev F required more family
- *		information detection.
- *
- *	Changes/Fixes by Borislav Petkov <bp@alien8.de>:
- *		- misc fixes and code cleanups
- *
- * This module is based on the following documents
- * (available from http://www.amd.com/):
- *
- *	Title:	BIOS and Kernel Developer's Guide for AMD Athlon 64 and AMD
- *		Opteron Processors
- *	AMD publication #: 26094
- *`	Revision: 3.26
- *
- *	Title:	BIOS and Kernel Developer's Guide for AMD NPT Family 0Fh
- *		Processors
- *	AMD publication #: 32559
- *	Revision: 3.00
- *	Issue Date: May 2006
- *
- *	Title:	BIOS and Kernel Developer's Guide (BKDG) For AMD Family 10h
- *		Processors
- *	AMD publication #: 31116
- *	Revision: 3.00
- *	Issue Date: September 07, 2007
- *
- * Sections in the first 2 documents are no longer in sync with each other.
- * The Family 10h BKDG was totally re-written from scratch with a new
- * presentation model.
- * Therefore, comments that refer to a Document section might be off.
  */
 
 #include <linux/module.h>
-- 
2.4.0

Re: [PATCH V2 1/3] EDAC, amd64_edac: Extend scrub rate programmability feature for F15hM60h

[Borislav Petkov]

On Mon, Sep 28, 2015 at 06:43:12AM -0500, Aravind Gopalakrishnan wrote:
> For F15h M60h processor, the scrub rate control register has moved
> to F2 of PCI config space and is at a different offset from
> earlier processors. The minimun recommended scrub rate is also different.
> (Refer D18F2x1c9_dct[1:0][DramScrub] on Fam15hM60h BKDG)
> 
> Modify the set_scrub_rate() and get_scrub_rate() functions so that
> they are aware of these changes.
> 
> Tested on F15hM60h, Fam15h Models 00h-0fh and Fam10h systems and
> it works fine.
> 
> Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
> ---
>  drivers/edac/amd64_edac.c | 23 +++++++++++++++++++----
>  drivers/edac/amd64_edac.h |  2 ++
>  2 files changed, 21 insertions(+), 4 deletions(-)

...

> @@ -216,12 +223,17 @@ static int set_scrub_rate(struct mem_ctl_info *mci, u32 bw)
>  
>  	if (pvt->fam == 0xf)
>  		min_scrubrate = 0x0;
> +	else if (pvt->fam == 0x15 && pvt->model == 0x60)
> +		min_scrubrate = 0x6;
>  
>  	/* Erratum #505 */
>  	if (pvt->fam == 0x15 && pvt->model < 0x10)
>  		f15h_select_dct(pvt, 0);
>  
> -	return __set_scrub_rate(pvt->F3, bw, min_scrubrate);
> +	if (pvt->fam == 0x15 && pvt->model == 0x60)
> +		return __set_scrub_rate(pvt, bw, min_scrubrate);
> +
> +	return __set_scrub_rate(pvt, bw, min_scrubrate);

This looks sloppy:

	if (condition)
		return function()
	return function()

I ended up committing the version below. Other two applied too.

---
From: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
Date: Mon, 28 Sep 2015 06:43:12 -0500
Subject: [PATCH] EDAC, amd64_edac: Extend scrub rate support to F15hM60h

The scrub rate control register has moved to function 2 in PCI config
space and is at a different offset on family 0x15, models 0x60 and
later. The minimum recommended scrub rate has also changed. (Refer to
D18F2x1c9_dct[1:0][DramScrub] in Fam15hM60h BKDG).

Adjust set_scrub_rate() and get_scrub_rate() functions to accommodate
this.

Tested on F15hM60h, Fam15h, models 00h-0fh and Fam10h systems.

Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
Cc: linux-edac <linux-edac@vger.kernel.org>
Link: http://lkml.kernel.org/r/1443440593-2316-2-git-send-email-Aravind.Gopalakrishnan@amd.com
[ Cleanup conditionals. ]
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 drivers/edac/amd64_edac.c | 35 +++++++++++++++++++++++++----------
 drivers/edac/amd64_edac.h |  2 ++
 2 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 73aea40a9c89..ca03a736b106 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -173,7 +173,7 @@ static inline int amd64_read_dct_pci_cfg(struct amd64_pvt *pvt, u8 dct,
  * scan the scrub rate mapping table for a close or matching bandwidth value to
  * issue. If requested is too big, then use last maximum value found.
  */
-static int __set_scrub_rate(struct pci_dev *ctl, u32 new_bw, u32 min_rate)
+static int __set_scrub_rate(struct amd64_pvt *pvt, u32 new_bw, u32 min_rate)
 {
 	u32 scrubval;
 	int i;
@@ -201,7 +201,14 @@ static int __set_scrub_rate(struct pci_dev *ctl, u32 new_bw, u32 min_rate)
 
 	scrubval = scrubrates[i].scrubval;
 
-	pci_write_bits32(ctl, SCRCTRL, scrubval, 0x001F);
+	if (pvt->fam == 0x15 && pvt->model == 0x60) {
+		f15h_select_dct(pvt, 0);
+		pci_write_bits32(pvt->F2, F15H_M60H_SCRCTRL, scrubval, 0x001F);
+		f15h_select_dct(pvt, 1);
+		pci_write_bits32(pvt->F2, F15H_M60H_SCRCTRL, scrubval, 0x001F);
+	} else {
+		pci_write_bits32(pvt->F3, SCRCTRL, scrubval, 0x001F);
+	}
 
 	if (scrubval)
 		return scrubrates[i].bandwidth;
@@ -217,11 +224,15 @@ static int set_scrub_rate(struct mem_ctl_info *mci, u32 bw)
 	if (pvt->fam == 0xf)
 		min_scrubrate = 0x0;
 
-	/* Erratum #505 */
-	if (pvt->fam == 0x15 && pvt->model < 0x10)
-		f15h_select_dct(pvt, 0);
+	if (pvt->fam == 0x15) {
+		/* Erratum #505 */
+		if (pvt->model < 0x10)
+			f15h_select_dct(pvt, 0);
 
-	return __set_scrub_rate(pvt->F3, bw, min_scrubrate);
+		if (pvt->model == 0x60)
+			min_scrubrate = 0x6;
+	}
+	return __set_scrub_rate(pvt, bw, min_scrubrate);
 }
 
 static int get_scrub_rate(struct mem_ctl_info *mci)
@@ -230,11 +241,15 @@ static int get_scrub_rate(struct mem_ctl_info *mci)
 	u32 scrubval = 0;
 	int i, retval = -EINVAL;
 
-	/* Erratum #505 */
-	if (pvt->fam == 0x15 && pvt->model < 0x10)
-		f15h_select_dct(pvt, 0);
+	if (pvt->fam == 0x15) {
+		/* Erratum #505 */
+		if (pvt->model < 0x10)
+			f15h_select_dct(pvt, 0);
 
-	amd64_read_pci_cfg(pvt->F3, SCRCTRL, &scrubval);
+		if (pvt->model == 0x60)
+			amd64_read_pci_cfg(pvt->F2, F15H_M60H_SCRCTRL, &scrubval);
+	} else
+		amd64_read_pci_cfg(pvt->F3, SCRCTRL, &scrubval);
 
 	scrubval = scrubval & 0x001F;
 
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index 4bdec752d330..971dc12a0a1c 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -255,6 +255,8 @@
 
 #define DCT_SEL_HI			0x114
 
+#define F15H_M60H_SCRCTRL		0x1C8
+
 /*
  * Function 3 - Misc Control
  */
-- 
2.3.5

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

Re: [PATCH V2 1/3] EDAC, amd64_edac: Extend scrub rate programmability feature for F15hM60h

[Aravind Gopalakrishnan]

On 9/29/2015 6:45 AM, Borislav Petkov wrote:
>> @@ -216,12 +223,17 @@ static int set_scrub_rate(struct mem_ctl_info *mci, u32 bw)
>>   
>>   	if (pvt->fam == 0xf)
>>   		min_scrubrate = 0x0;
>> +	else if (pvt->fam == 0x15 && pvt->model == 0x60)
>> +		min_scrubrate = 0x6;
>>   
>>   	/* Erratum #505 */
>>   	if (pvt->fam == 0x15 && pvt->model < 0x10)
>>   		f15h_select_dct(pvt, 0);
>>   
>> -	return __set_scrub_rate(pvt->F3, bw, min_scrubrate);
>> +	if (pvt->fam == 0x15 && pvt->model == 0x60)
>> +		return __set_scrub_rate(pvt, bw, min_scrubrate);
>> +
>> +	return __set_scrub_rate(pvt, bw, min_scrubrate);
> This looks sloppy:
>
> 	if (condition)
> 		return function()
> 	return function()
>
> I ended up committing the version below. Other two applied too.
>

Forgot to remove that conditional :|
Thanks for cleaning it up.

-Aravind.

>   
>
> +	if (pvt->fam == 0x15) {
> +		/* Erratum #505 */
> +		if (pvt->model < 0x10)
> +			f15h_select_dct(pvt, 0);
>   
> -	return __set_scrub_rate(pvt->F3, bw, min_scrubrate);
> +		if (pvt->model == 0x60)
> +			min_scrubrate = 0x6;
> +	}
> +	return __set_scrub_rate(pvt, bw, min_scrubrate);
>   }
>   
>   static int get_scrub_rate(struct mem_ctl_info *mci)
> @@ -230,11 +241,15 @@ static int get_scrub_rate(struct mem_ctl_info *mci)
>   	u32 scrubval = 0;
>   	int i, retval = -EINVAL;
>   
>
> +	if (pvt->fam == 0x15) {
> +		/* Erratum #505 */
> +		if (pvt->model < 0x10)
> +			f15h_select_dct(pvt, 0);
>   
> -	amd64_read_pci_cfg(pvt->F3, SCRCTRL, &scrubval);
> +		if (pvt->model == 0x60)
> +			amd64_read_pci_cfg(pvt->F2, F15H_M60H_SCRCTRL, &scrubval);
> +	} else
> +		amd64_read_pci_cfg(pvt->F3, SCRCTRL, &scrubval);
>   
>   	scrubval = scrubval & 0x001F;
>   
>