2012-10-31 05:56:08

by Daniel J Blueman

[permalink] [raw]
Subject: [PATCH 1/4, v4] AMD64 EDAC: Add multi-domain support to AMD EDAC

Fix the handling of memory controller detection to index the array
of detected Northbridges, allowing memory controllers over multiple
PCI domains in federated systems eg using Numascale's NumaConnect/
NumaChip.

v4: Generate linear Northbridge ID by indexing detected Northbridges

Signed-off-by: Daniel J Blueman <[email protected]>
---
arch/x86/include/asm/amd_nb.h | 12 ++++++++++++
drivers/edac/amd64_edac.c | 18 ++++++++++++++----
drivers/edac/amd64_edac.h | 6 ------
3 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/amd_nb.h b/arch/x86/include/asm/amd_nb.h
index b3341e9..b88fc7a 100644
--- a/arch/x86/include/asm/amd_nb.h
+++ b/arch/x86/include/asm/amd_nb.h
@@ -81,6 +81,19 @@ static inline struct amd_northbridge *node_to_amd_nb(int node)
return (node < amd_northbridges.num) ? &amd_northbridges.nb[node] : NULL;
}

+static inline u16 get_node_id(struct pci_dev *pdev)
+{
+ int i;
+
+ for (i = 0; i != amd_nb_num(); i++)
+ if (pci_domain_nr(node_to_amd_nb(i)->misc->bus) == pci_domain_nr(pdev->bus) &&
+ PCI_SLOT(node_to_amd_nb(i)->misc->devfn) == PCI_SLOT(pdev->devfn))
+ return i;
+
+ WARN(1, "Unable to find AMD Northbridge identifier\n");
+ return 0;
+}
+
#else

#define amd_nb_num(x) 0
diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index cc8e7c7..18d404a 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -982,6 +982,9 @@ static u64 get_error_address(struct mce *m)
return addr;
}

+static struct amd64_family_type *amd64_per_family_init(struct amd64_pvt *pvt);
+static struct pci_dev *pci_get_related_function(unsigned int vendor, unsigned int device, struct pci_dev *related);
+
static void read_dram_base_limit_regs(struct amd64_pvt *pvt, unsigned range)
{
struct cpuinfo_x86 *c = &boot_cpu_data;
@@ -1001,11 +1004,17 @@ static void read_dram_base_limit_regs(struct amd64_pvt *pvt, unsigned range)

/* Factor in CC6 save area by reading dst node's limit reg */
if (c->x86 == 0x15) {
- struct pci_dev *f1 = NULL;
- u8 nid = dram_dst_node(pvt, range);
+ struct pci_dev *misc, *f1 = NULL;
+ struct amd64_family_type *fam_type;
+ u16 nid = dram_dst_node(pvt, range);
u32 llim;

- f1 = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0x18 + nid, 1));
+ misc = node_to_amd_nb(nid)->misc;
+ fam_type = amd64_per_family_init(pvt);
+ if (WARN_ON(!f1))
+ return;
+
+ f1 = pci_get_related_function(misc->vendor, fam_type->f1_id, misc);
if (WARN_ON(!f1))
return;

@@ -1720,7 +1729,8 @@ static struct pci_dev *pci_get_related_function(unsigned int vendor,

dev = pci_get_device(vendor, device, dev);
while (dev) {
- if ((dev->bus->number == related->bus->number) &&
+ if (pci_domain_nr(dev->bus) == pci_domain_nr(related->bus) &&
+ (dev->bus->number == related->bus->number) &&
(PCI_SLOT(dev->devfn) == PCI_SLOT(related->devfn)))
break;
dev = pci_get_device(vendor, device, dev);
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index 8d48047..90cae61 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -290,12 +290,6 @@
/* MSRs */
#define MSR_MCGCTL_NBE BIT(4)

-/* AMD sets the first MC device at device ID 0x18. */
-static inline u8 get_node_id(struct pci_dev *pdev)
-{
- return PCI_SLOT(pdev->devfn) - 0x18;
-}
-
enum amd_families {
K8_CPUS = 0,
F10_CPUS,
--
1.7.9.5


2012-10-31 05:56:18

by Daniel J Blueman

[permalink] [raw]
Subject: [PATCH 2/4] AMD64 EDAC: Add support for >255 memory controllers

As the AMD64 last-level-cache ID is 16-bits and federated systems
eg using Numascale's NumaConnect/NumaChip can have more than 255 memory
controllers, use 16-bits to store the ID.

Signed-off-by: Daniel J Blueman <[email protected]>
---
drivers/edac/amd64_edac.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 18d404a..9920dfd 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -942,7 +942,7 @@ static u64 get_error_address(struct mce *m)
struct amd64_pvt *pvt;
u64 cc6_base, tmp_addr;
u32 tmp;
- u8 mce_nid, intlv_en;
+ u16 mce_nid, intlv_en;

if ((addr & GENMASK(24, 47)) >> 24 != 0x00fdf7)
return addr;
@@ -1499,7 +1499,7 @@ static int f1x_match_to_this_node(struct amd64_pvt *pvt, unsigned range,
u8 channel;
bool high_range = false;

- u8 node_id = dram_dst_node(pvt, range);
+ u16 node_id = dram_dst_node(pvt, range);
u8 intlv_en = dram_intlv_en(pvt, range);
u32 intlv_sel = dram_intlv_sel(pvt, range);

@@ -2306,7 +2306,7 @@ out:
return ret;
}

-static int toggle_ecc_err_reporting(struct ecc_settings *s, u8 nid, bool on)
+static int toggle_ecc_err_reporting(struct ecc_settings *s, u16 nid, bool on)
{
cpumask_var_t cmask;
int cpu;
@@ -2344,7 +2344,7 @@ static int toggle_ecc_err_reporting(struct ecc_settings *s, u8 nid, bool on)
return 0;
}

-static bool enable_ecc_error_reporting(struct ecc_settings *s, u8 nid,
+static bool enable_ecc_error_reporting(struct ecc_settings *s, u16 nid,
struct pci_dev *F3)
{
bool ret = true;
@@ -2396,7 +2396,7 @@ static bool enable_ecc_error_reporting(struct ecc_settings *s, u8 nid,
return ret;
}

-static void restore_ecc_error_reporting(struct ecc_settings *s, u8 nid,
+static void restore_ecc_error_reporting(struct ecc_settings *s, u16 nid,
struct pci_dev *F3)
{
u32 value, mask = 0x3; /* UECC/CECC enable */
@@ -2435,7 +2435,7 @@ static const char *ecc_msg =
"'ecc_enable_override'.\n"
" (Note that use of the override may cause unknown side effects.)\n";

-static bool ecc_enabled(struct pci_dev *F3, u8 nid)
+static bool ecc_enabled(struct pci_dev *F3, u16 nid)
{
u32 value;
u8 ecc_en = 0;
@@ -2556,7 +2556,7 @@ static int amd64_init_one_instance(struct pci_dev *F2)
struct mem_ctl_info *mci = NULL;
struct edac_mc_layer layers[2];
int err = 0, ret;
- u8 nid = get_node_id(F2);
+ u16 nid = get_node_id(F2);

ret = -ENOMEM;
pvt = kzalloc(sizeof(struct amd64_pvt), GFP_KERNEL);
@@ -2647,7 +2647,7 @@ err_ret:
static int __devinit amd64_probe_one_instance(struct pci_dev *pdev,
const struct pci_device_id *mc_type)
{
- u8 nid = get_node_id(pdev);
+ u16 nid = get_node_id(pdev);
struct pci_dev *F3 = node_to_amd_nb(nid)->misc;
struct ecc_settings *s;
int ret = 0;
@@ -2697,7 +2697,7 @@ static void __devexit amd64_remove_one_instance(struct pci_dev *pdev)
{
struct mem_ctl_info *mci;
struct amd64_pvt *pvt;
- u8 nid = get_node_id(pdev);
+ u16 nid = get_node_id(pdev);
struct pci_dev *F3 = node_to_amd_nb(nid)->misc;
struct ecc_settings *s = ecc_stngs[nid];

--
1.7.9.5

2012-10-31 05:56:24

by Daniel J Blueman

[permalink] [raw]
Subject: [PATCH 3/4] AMD64 EDAC: Cleanup type usage to be consistent

As the Northbridge IDs are at most 16-bits, use the same type
consistently.

Signed-off-by: Daniel J Blueman <[email protected]>
---
arch/x86/include/asm/amd_nb.h | 2 +-
arch/x86/include/asm/processor.h | 2 +-
arch/x86/kernel/cpu/amd.c | 4 ++--
drivers/edac/amd64_edac.c | 26 ++++++++++++++------------
drivers/edac/amd64_edac.h | 2 +-
5 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/arch/x86/include/asm/amd_nb.h b/arch/x86/include/asm/amd_nb.h
index b88fc7a..0cc1045 100644
--- a/arch/x86/include/asm/amd_nb.h
+++ b/arch/x86/include/asm/amd_nb.h
@@ -76,7 +76,7 @@ static inline bool amd_nb_has_feature(unsigned feature)
return ((amd_northbridges.flags & feature) == feature);
}

-static inline struct amd_northbridge *node_to_amd_nb(int node)
+static inline struct amd_northbridge *node_to_amd_nb(u16 node)
{
return (node < amd_northbridges.num) ? &amd_northbridges.nb[node] : NULL;
}
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index ad1fc85..eb3ba58 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -934,7 +934,7 @@ extern void start_thread(struct pt_regs *regs, unsigned long new_ip,
extern int get_tsc_mode(unsigned long adr);
extern int set_tsc_mode(unsigned int val);

-extern int amd_get_nb_id(int cpu);
+extern u16 amd_get_nb_id(int cpu);

struct aperfmperf {
u64 aperf, mperf;
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index f7e98a2..52cab1f 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -364,9 +364,9 @@ static void __cpuinit amd_detect_cmp(struct cpuinfo_x86 *c)
#endif
}

-int amd_get_nb_id(int cpu)
+u16 amd_get_nb_id(int cpu)
{
- int id = 0;
+ u16 id = 0;
#ifdef CONFIG_SMP
id = per_cpu(cpu_llc_id, cpu);
#endif
diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 9920dfd..12cd675 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -239,7 +239,7 @@ static int amd64_get_scrub_rate(struct mem_ctl_info *mci)
* DRAM base/limit associated with node_id
*/
static bool amd64_base_limit_match(struct amd64_pvt *pvt, u64 sys_addr,
- unsigned nid)
+ u16 nid)
{
u64 addr;

@@ -265,7 +265,7 @@ static struct mem_ctl_info *find_mc_by_sys_addr(struct mem_ctl_info *mci,
u64 sys_addr)
{
struct amd64_pvt *pvt;
- unsigned node_id;
+ u16 node_id;
u32 intlv_en, bits;

/*
@@ -613,7 +613,8 @@ static u64 sys_addr_to_input_addr(struct mem_ctl_info *mci, u64 sys_addr)
static u64 input_addr_to_dram_addr(struct mem_ctl_info *mci, u64 input_addr)
{
struct amd64_pvt *pvt;
- unsigned node_id, intlv_shift;
+ u16 node_id;
+ unsigned intlv_shift;
u64 bits, dram_addr;
u32 intlv_sel;

@@ -1337,7 +1338,7 @@ static u8 f1x_determine_channel(struct amd64_pvt *pvt, u64 sys_addr,
}

/* Convert the sys_addr to the normalized DCT address */
-static u64 f1x_get_norm_dct_addr(struct amd64_pvt *pvt, unsigned range,
+static u64 f1x_get_norm_dct_addr(struct amd64_pvt *pvt, u16 range,
u64 sys_addr, bool hi_rng,
u32 dct_sel_base_addr)
{
@@ -1413,7 +1414,7 @@ static int f10_process_possible_spare(struct amd64_pvt *pvt, u8 dct, int csrow)
* -EINVAL: NOT FOUND
* 0..csrow = Chip-Select Row
*/
-static int f1x_lookup_addr_in_dct(u64 in_addr, u32 nid, u8 dct)
+static int f1x_lookup_addr_in_dct(u64 in_addr, u16 nid, u8 dct)
{
struct mem_ctl_info *mci;
struct amd64_pvt *pvt;
@@ -1491,7 +1492,7 @@ static u64 f1x_swap_interleaved_region(struct amd64_pvt *pvt, u64 sys_addr)

/* For a given @dram_range, check if @sys_addr falls within it. */
static int f1x_match_to_this_node(struct amd64_pvt *pvt, unsigned range,
- u64 sys_addr, int *nid, int *chan_sel)
+ u64 sys_addr, u16 *nid, int *chan_sel)
{
int cs_found = -EINVAL;
u64 chan_addr;
@@ -1572,10 +1573,10 @@ static int f1x_match_to_this_node(struct amd64_pvt *pvt, unsigned range,
}

static int f1x_translate_sysaddr_to_cs(struct amd64_pvt *pvt, u64 sys_addr,
- int *node, int *chan_sel)
+ u16 *node, int *chan_sel)
{
int cs_found = -EINVAL;
- unsigned range;
+ u16 range;

for (range = 0; range < DRAM_RANGES; range++) {

@@ -1607,7 +1608,8 @@ static void f1x_map_sysaddr_to_csrow(struct mem_ctl_info *mci, u64 sys_addr,
{
struct amd64_pvt *pvt = mci->pvt_info;
u32 page, offset;
- int nid, csrow, chan = 0;
+ int csrow, chan = 0;
+ u16 nid;

error_address_to_page_and_offset(sys_addr, &page, &offset);

@@ -2065,7 +2067,7 @@ static void read_mc_regs(struct amd64_pvt *pvt)
struct cpuinfo_x86 *c = &boot_cpu_data;
u64 msr_val;
u32 tmp;
- unsigned range;
+ u16 range;

/*
* Retrieve TOP_MEM and TOP_MEM2; no masking off of reserved bits since
@@ -2263,7 +2265,7 @@ static int init_csrows(struct mem_ctl_info *mci)
}

/* get all cores on this DCT */
-static void get_cpus_on_this_dct_cpumask(struct cpumask *mask, unsigned nid)
+static void get_cpus_on_this_dct_cpumask(struct cpumask *mask, u16 nid)
{
int cpu;

@@ -2273,7 +2275,7 @@ static void get_cpus_on_this_dct_cpumask(struct cpumask *mask, unsigned nid)
}

/* check MCG_CTL on all the cpus on this node */
-static bool amd64_nb_mce_bank_enabled_on_node(unsigned nid)
+static bool amd64_nb_mce_bank_enabled_on_node(u16 nid)
{
cpumask_var_t mask;
int cpu, nbe;
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index 90cae61..647c9b8 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -332,7 +332,7 @@ struct amd64_pvt {
/* pci_device handles which we utilize */
struct pci_dev *F1, *F2, *F3;

- unsigned mc_node_id; /* MC index of this MC node */
+ u16 mc_node_id; /* MC index of this MC node */
int ext_model; /* extended model value of this node */
int channel_count;

--
1.7.9.5

2012-10-31 05:56:32

by Daniel J Blueman

[permalink] [raw]
Subject: [PATCH 4/4] AMD64 EDAC: Use appropriate name for NB indexing

Use the same 'amd' prefix as related functions for clarity.

Signed-off-by: Daniel J Blueman <[email protected]>
---
arch/x86/include/asm/amd_nb.h | 2 +-
drivers/edac/amd64_edac.c | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/amd_nb.h b/arch/x86/include/asm/amd_nb.h
index 0cc1045..39b5ddd 100644
--- a/arch/x86/include/asm/amd_nb.h
+++ b/arch/x86/include/asm/amd_nb.h
@@ -81,7 +81,7 @@ static inline struct amd_northbridge *node_to_amd_nb(u16 node)
return (node < amd_northbridges.num) ? &amd_northbridges.nb[node] : NULL;
}

-static inline u16 get_node_id(struct pci_dev *pdev)
+static inline u16 amd_get_node_id(struct pci_dev *pdev)
{
int i;

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 12cd675..59658b9 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -2558,7 +2558,7 @@ static int amd64_init_one_instance(struct pci_dev *F2)
struct mem_ctl_info *mci = NULL;
struct edac_mc_layer layers[2];
int err = 0, ret;
- u16 nid = get_node_id(F2);
+ u16 nid = amd_get_node_id(F2);

ret = -ENOMEM;
pvt = kzalloc(sizeof(struct amd64_pvt), GFP_KERNEL);
@@ -2649,7 +2649,7 @@ err_ret:
static int __devinit amd64_probe_one_instance(struct pci_dev *pdev,
const struct pci_device_id *mc_type)
{
- u16 nid = get_node_id(pdev);
+ u16 nid = amd_get_node_id(pdev);
struct pci_dev *F3 = node_to_amd_nb(nid)->misc;
struct ecc_settings *s;
int ret = 0;
@@ -2699,7 +2699,7 @@ static void __devexit amd64_remove_one_instance(struct pci_dev *pdev)
{
struct mem_ctl_info *mci;
struct amd64_pvt *pvt;
- u16 nid = get_node_id(pdev);
+ u16 nid = amd_get_node_id(pdev);
struct pci_dev *F3 = node_to_amd_nb(nid)->misc;
struct ecc_settings *s = ecc_stngs[nid];

--
1.7.9.5

2012-10-31 08:18:33

by Torsten Kaiser

[permalink] [raw]
Subject: Re: [PATCH 2/4] AMD64 EDAC: Add support for >255 memory controllers

On Wed, Oct 31, 2012 at 6:55 AM, Daniel J Blueman
<[email protected]> wrote:
> As the AMD64 last-level-cache ID is 16-bits and federated systems
> eg using Numascale's NumaConnect/NumaChip can have more than 255 memory
> controllers, use 16-bits to store the ID.
>
> Signed-off-by: Daniel J Blueman <[email protected]>
> ---
> drivers/edac/amd64_edac.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index 18d404a..9920dfd 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -942,7 +942,7 @@ static u64 get_error_address(struct mce *m)
> struct amd64_pvt *pvt;
> u64 cc6_base, tmp_addr;
> u32 tmp;
> - u8 mce_nid, intlv_en;
> + u16 mce_nid, intlv_en;

Is the change of intlv_en to u16 intentional?
I assume its not, because...

> if ((addr & GENMASK(24, 47)) >> 24 != 0x00fdf7)
> return addr;
> @@ -1499,7 +1499,7 @@ static int f1x_match_to_this_node(struct amd64_pvt *pvt, unsigned range,
> u8 channel;
> bool high_range = false;
>
> - u8 node_id = dram_dst_node(pvt, range);
> + u16 node_id = dram_dst_node(pvt, range);
> u8 intlv_en = dram_intlv_en(pvt, range);

... here you keep it at u8.

> u32 intlv_sel = dram_intlv_sel(pvt, range);
>
> @@ -2306,7 +2306,7 @@ out:
> return ret;
> }
>
> -static int toggle_ecc_err_reporting(struct ecc_settings *s, u8 nid, bool on)
> +static int toggle_ecc_err_reporting(struct ecc_settings *s, u16 nid, bool on)
> {
> cpumask_var_t cmask;
> int cpu;
> @@ -2344,7 +2344,7 @@ static int toggle_ecc_err_reporting(struct ecc_settings *s, u8 nid, bool on)
> return 0;
> }
>
> -static bool enable_ecc_error_reporting(struct ecc_settings *s, u8 nid,
> +static bool enable_ecc_error_reporting(struct ecc_settings *s, u16 nid,
> struct pci_dev *F3)
> {
> bool ret = true;
> @@ -2396,7 +2396,7 @@ static bool enable_ecc_error_reporting(struct ecc_settings *s, u8 nid,
> return ret;
> }
>
> -static void restore_ecc_error_reporting(struct ecc_settings *s, u8 nid,
> +static void restore_ecc_error_reporting(struct ecc_settings *s, u16 nid,
> struct pci_dev *F3)
> {
> u32 value, mask = 0x3; /* UECC/CECC enable */
> @@ -2435,7 +2435,7 @@ static const char *ecc_msg =
> "'ecc_enable_override'.\n"
> " (Note that use of the override may cause unknown side effects.)\n";
>
> -static bool ecc_enabled(struct pci_dev *F3, u8 nid)
> +static bool ecc_enabled(struct pci_dev *F3, u16 nid)
> {
> u32 value;
> u8 ecc_en = 0;
> @@ -2556,7 +2556,7 @@ static int amd64_init_one_instance(struct pci_dev *F2)
> struct mem_ctl_info *mci = NULL;
> struct edac_mc_layer layers[2];
> int err = 0, ret;
> - u8 nid = get_node_id(F2);
> + u16 nid = get_node_id(F2);
>
> ret = -ENOMEM;
> pvt = kzalloc(sizeof(struct amd64_pvt), GFP_KERNEL);
> @@ -2647,7 +2647,7 @@ err_ret:
> static int __devinit amd64_probe_one_instance(struct pci_dev *pdev,
> const struct pci_device_id *mc_type)
> {
> - u8 nid = get_node_id(pdev);
> + u16 nid = get_node_id(pdev);
> struct pci_dev *F3 = node_to_amd_nb(nid)->misc;
> struct ecc_settings *s;
> int ret = 0;
> @@ -2697,7 +2697,7 @@ static void __devexit amd64_remove_one_instance(struct pci_dev *pdev)
> {
> struct mem_ctl_info *mci;
> struct amd64_pvt *pvt;
> - u8 nid = get_node_id(pdev);
> + u16 nid = get_node_id(pdev);
> struct pci_dev *F3 = node_to_amd_nb(nid)->misc;
> struct ecc_settings *s = ecc_stngs[nid];
>
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2012-10-31 08:20:28

by Daniel J Blueman

[permalink] [raw]
Subject: Re: [PATCH 2/4] AMD64 EDAC: Add support for >255 memory controllers

On 31/10/2012 16:18, Torsten Kaiser wrote:
> On Wed, Oct 31, 2012 at 6:55 AM, Daniel J Blueman
> <[email protected]> wrote:
>> As the AMD64 last-level-cache ID is 16-bits and federated systems
>> eg using Numascale's NumaConnect/NumaChip can have more than 255 memory
>> controllers, use 16-bits to store the ID.
>>
>> Signed-off-by: Daniel J Blueman <[email protected]>
>> ---
>> drivers/edac/amd64_edac.c | 18 +++++++++---------
>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
>> index 18d404a..9920dfd 100644
>> --- a/drivers/edac/amd64_edac.c
>> +++ b/drivers/edac/amd64_edac.c
>> @@ -942,7 +942,7 @@ static u64 get_error_address(struct mce *m)
>> struct amd64_pvt *pvt;
>> u64 cc6_base, tmp_addr;
>> u32 tmp;
>> - u8 mce_nid, intlv_en;
>> + u16 mce_nid, intlv_en;
>
> Is the change of intlv_en to u16 intentional?
> I assume its not, because...

It's unintentional. Elsewhere, intlv_en is declared as unsigned, so
perhaps that should be cleaned up later too.

I'll issue an updated patch.

>> if ((addr & GENMASK(24, 47)) >> 24 != 0x00fdf7)
>> return addr;
>> @@ -1499,7 +1499,7 @@ static int f1x_match_to_this_node(struct amd64_pvt *pvt, unsigned range,
>> u8 channel;
>> bool high_range = false;
>>
>> - u8 node_id = dram_dst_node(pvt, range);
>> + u16 node_id = dram_dst_node(pvt, range);
>> u8 intlv_en = dram_intlv_en(pvt, range);
>
> ... here you keep it at u8.
>
>> u32 intlv_sel = dram_intlv_sel(pvt, range);
>>
>> @@ -2306,7 +2306,7 @@ out:
>> return ret;
>> }
>>
>> -static int toggle_ecc_err_reporting(struct ecc_settings *s, u8 nid, bool on)
>> +static int toggle_ecc_err_reporting(struct ecc_settings *s, u16 nid, bool on)
>> {
>> cpumask_var_t cmask;
>> int cpu;
>> @@ -2344,7 +2344,7 @@ static int toggle_ecc_err_reporting(struct ecc_settings *s, u8 nid, bool on)
>> return 0;
>> }
>>
>> -static bool enable_ecc_error_reporting(struct ecc_settings *s, u8 nid,
>> +static bool enable_ecc_error_reporting(struct ecc_settings *s, u16 nid,
>> struct pci_dev *F3)
>> {
>> bool ret = true;
>> @@ -2396,7 +2396,7 @@ static bool enable_ecc_error_reporting(struct ecc_settings *s, u8 nid,
>> return ret;
>> }
>>
>> -static void restore_ecc_error_reporting(struct ecc_settings *s, u8 nid,
>> +static void restore_ecc_error_reporting(struct ecc_settings *s, u16 nid,
>> struct pci_dev *F3)
>> {
>> u32 value, mask = 0x3; /* UECC/CECC enable */
>> @@ -2435,7 +2435,7 @@ static const char *ecc_msg =
>> "'ecc_enable_override'.\n"
>> " (Note that use of the override may cause unknown side effects.)\n";
>>
>> -static bool ecc_enabled(struct pci_dev *F3, u8 nid)
>> +static bool ecc_enabled(struct pci_dev *F3, u16 nid)
>> {
>> u32 value;
>> u8 ecc_en = 0;
>> @@ -2556,7 +2556,7 @@ static int amd64_init_one_instance(struct pci_dev *F2)
>> struct mem_ctl_info *mci = NULL;
>> struct edac_mc_layer layers[2];
>> int err = 0, ret;
>> - u8 nid = get_node_id(F2);
>> + u16 nid = get_node_id(F2);
>>
>> ret = -ENOMEM;
>> pvt = kzalloc(sizeof(struct amd64_pvt), GFP_KERNEL);
>> @@ -2647,7 +2647,7 @@ err_ret:
>> static int __devinit amd64_probe_one_instance(struct pci_dev *pdev,
>> const struct pci_device_id *mc_type)
>> {
>> - u8 nid = get_node_id(pdev);
>> + u16 nid = get_node_id(pdev);
>> struct pci_dev *F3 = node_to_amd_nb(nid)->misc;
>> struct ecc_settings *s;
>> int ret = 0;
>> @@ -2697,7 +2697,7 @@ static void __devexit amd64_remove_one_instance(struct pci_dev *pdev)
>> {
>> struct mem_ctl_info *mci;
>> struct amd64_pvt *pvt;
>> - u8 nid = get_node_id(pdev);
>> + u16 nid = get_node_id(pdev);
>> struct pci_dev *F3 = node_to_amd_nb(nid)->misc;
>> struct ecc_settings *s = ecc_stngs[nid];
>>
>> --
>> 1.7.9.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/


--
Daniel J Blueman
Principal Software Engineer, Numascale Asia

2012-10-31 08:49:17

by Daniel J Blueman

[permalink] [raw]
Subject: [PATCH 2/4, v2] AMD64 EDAC: Add support for >255 memory controllers

As the AMD64 last-level-cache ID is 16-bits and federated systems
eg using Numascale's NumaConnect/NumaChip can have more than 255 memory
controllers, use 16-bits to store the ID.

v2: Avoid change to intlv_en variable

Signed-off-by: Daniel J Blueman <[email protected]>
---
drivers/edac/amd64_edac.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 18d404a..28b2005 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -942,7 +942,8 @@ static u64 get_error_address(struct mce *m)
struct amd64_pvt *pvt;
u64 cc6_base, tmp_addr;
u32 tmp;
- u8 mce_nid, intlv_en;
+ u16 mce_nid;
+ u8 intlv_en;

if ((addr & GENMASK(24, 47)) >> 24 != 0x00fdf7)
return addr;
@@ -1499,7 +1500,7 @@ static int f1x_match_to_this_node(struct amd64_pvt *pvt, unsigned range,
u8 channel;
bool high_range = false;

- u8 node_id = dram_dst_node(pvt, range);
+ u16 node_id = dram_dst_node(pvt, range);
u8 intlv_en = dram_intlv_en(pvt, range);
u32 intlv_sel = dram_intlv_sel(pvt, range);

@@ -2306,7 +2307,7 @@ out:
return ret;
}

-static int toggle_ecc_err_reporting(struct ecc_settings *s, u8 nid, bool on)
+static int toggle_ecc_err_reporting(struct ecc_settings *s, u16 nid, bool on)
{
cpumask_var_t cmask;
int cpu;
@@ -2344,7 +2345,7 @@ static int toggle_ecc_err_reporting(struct ecc_settings *s, u8 nid, bool on)
return 0;
}

-static bool enable_ecc_error_reporting(struct ecc_settings *s, u8 nid,
+static bool enable_ecc_error_reporting(struct ecc_settings *s, u16 nid,
struct pci_dev *F3)
{
bool ret = true;
@@ -2396,7 +2397,7 @@ static bool enable_ecc_error_reporting(struct ecc_settings *s, u8 nid,
return ret;
}

-static void restore_ecc_error_reporting(struct ecc_settings *s, u8 nid,
+static void restore_ecc_error_reporting(struct ecc_settings *s, u16 nid,
struct pci_dev *F3)
{
u32 value, mask = 0x3; /* UECC/CECC enable */
@@ -2435,7 +2436,7 @@ static const char *ecc_msg =
"'ecc_enable_override'.\n"
" (Note that use of the override may cause unknown side effects.)\n";

-static bool ecc_enabled(struct pci_dev *F3, u8 nid)
+static bool ecc_enabled(struct pci_dev *F3, u16 nid)
{
u32 value;
u8 ecc_en = 0;
@@ -2556,7 +2557,7 @@ static int amd64_init_one_instance(struct pci_dev *F2)
struct mem_ctl_info *mci = NULL;
struct edac_mc_layer layers[2];
int err = 0, ret;
- u8 nid = get_node_id(F2);
+ u16 nid = get_node_id(F2);

ret = -ENOMEM;
pvt = kzalloc(sizeof(struct amd64_pvt), GFP_KERNEL);
@@ -2647,7 +2648,7 @@ err_ret:
static int __devinit amd64_probe_one_instance(struct pci_dev *pdev,
const struct pci_device_id *mc_type)
{
- u8 nid = get_node_id(pdev);
+ u16 nid = get_node_id(pdev);
struct pci_dev *F3 = node_to_amd_nb(nid)->misc;
struct ecc_settings *s;
int ret = 0;
@@ -2697,7 +2698,7 @@ static void __devexit amd64_remove_one_instance(struct pci_dev *pdev)
{
struct mem_ctl_info *mci;
struct amd64_pvt *pvt;
- u8 nid = get_node_id(pdev);
+ u16 nid = get_node_id(pdev);
struct pci_dev *F3 = node_to_amd_nb(nid)->misc;
struct ecc_settings *s = ecc_stngs[nid];

--
1.7.9.5

2012-10-31 12:03:21

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 1/4, v4] AMD64 EDAC: Add multi-domain support to AMD EDAC

On Wed, Oct 31, 2012 at 01:55:29PM +0800, Daniel J Blueman wrote:
> Fix the handling of memory controller detection to index the array
> of detected Northbridges, allowing memory controllers over multiple
> PCI domains in federated systems eg using Numascale's NumaConnect/
> NumaChip.
>
> v4: Generate linear Northbridge ID by indexing detected Northbridges
>
> Signed-off-by: Daniel J Blueman <[email protected]>
> ---
> arch/x86/include/asm/amd_nb.h | 12 ++++++++++++
> drivers/edac/amd64_edac.c | 18 ++++++++++++++----
> drivers/edac/amd64_edac.h | 6 ------
> 3 files changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/arch/x86/include/asm/amd_nb.h b/arch/x86/include/asm/amd_nb.h
> index b3341e9..b88fc7a 100644
> --- a/arch/x86/include/asm/amd_nb.h
> +++ b/arch/x86/include/asm/amd_nb.h
> @@ -81,6 +81,19 @@ static inline struct amd_northbridge *node_to_amd_nb(int node)
> return (node < amd_northbridges.num) ? &amd_northbridges.nb[node] : NULL;
> }
>
> +static inline u16 get_node_id(struct pci_dev *pdev)

I'm guessing this'll be used in other code too, so rename it to belong
to the AMD namespace: amd_get_node_id.

> +{
> + int i;
> +
> + for (i = 0; i != amd_nb_num(); i++)
> + if (pci_domain_nr(node_to_amd_nb(i)->misc->bus) == pci_domain_nr(pdev->bus) &&
> + PCI_SLOT(node_to_amd_nb(i)->misc->devfn) == PCI_SLOT(pdev->devfn))
> + return i;

Ok, this relies on the fact that we always have the ->misc devices, i.e.
PCI F3. I guess that's ok.

> +
> + WARN(1, "Unable to find AMD Northbridge identifier\n");

WARN(1, "Unable to find AMD Northbridge identifier for %s\n", pci_name(pdev););

> + return 0;
> +}
> +
> #else
>
> #define amd_nb_num(x) 0
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index cc8e7c7..18d404a 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -982,6 +982,9 @@ static u64 get_error_address(struct mce *m)
> return addr;
> }
>
> +static struct amd64_family_type *amd64_per_family_init(struct amd64_pvt *pvt);
> +static struct pci_dev *pci_get_related_function(unsigned int vendor, unsigned int device, struct pci_dev *related);
> +
> static void read_dram_base_limit_regs(struct amd64_pvt *pvt, unsigned range)
> {
> struct cpuinfo_x86 *c = &boot_cpu_data;
> @@ -1001,11 +1004,17 @@ static void read_dram_base_limit_regs(struct amd64_pvt *pvt, unsigned range)
>
> /* Factor in CC6 save area by reading dst node's limit reg */
> if (c->x86 == 0x15) {
> - struct pci_dev *f1 = NULL;
> - u8 nid = dram_dst_node(pvt, range);
> + struct pci_dev *misc, *f1 = NULL;
> + struct amd64_family_type *fam_type;
> + u16 nid = dram_dst_node(pvt, range);
> u32 llim;
>
> - f1 = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0x18 + nid, 1));
> + misc = node_to_amd_nb(nid)->misc;
> + fam_type = amd64_per_family_init(pvt);

No need for fam_type here since this is F15h-only code, simply use
PCI_DEVICE_ID_AMD_15H_NB_F1 in the function below.

> + if (WARN_ON(!f1))
> + return;
> +
> + f1 = pci_get_related_function(misc->vendor, fam_type->f1_id, misc);
> if (WARN_ON(!f1))
> return;
>
> @@ -1720,7 +1729,8 @@ static struct pci_dev *pci_get_related_function(unsigned int vendor,
>
> dev = pci_get_device(vendor, device, dev);
> while (dev) {
> - if ((dev->bus->number == related->bus->number) &&
> + if (pci_domain_nr(dev->bus) == pci_domain_nr(related->bus) &&
> + (dev->bus->number == related->bus->number) &&
> (PCI_SLOT(dev->devfn) == PCI_SLOT(related->devfn)))
> break;
> dev = pci_get_device(vendor, device, dev);
> diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
> index 8d48047..90cae61 100644
> --- a/drivers/edac/amd64_edac.h
> +++ b/drivers/edac/amd64_edac.h
> @@ -290,12 +290,6 @@
> /* MSRs */
> #define MSR_MCGCTL_NBE BIT(4)
>
> -/* AMD sets the first MC device at device ID 0x18. */
> -static inline u8 get_node_id(struct pci_dev *pdev)
> -{
> - return PCI_SLOT(pdev->devfn) - 0x18;
> -}
> -
> enum amd_families {
> K8_CPUS = 0,
> F10_CPUS,
> --
> 1.7.9.5
>
>

--
Regards/Gruss,
Boris.

2012-10-31 18:55:35

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 2/4, v2] AMD64 EDAC: Add support for >255 memory controllers

On Wed, Oct 31, 2012 at 04:48:12PM +0800, Daniel J Blueman wrote:
> As the AMD64 last-level-cache ID is 16-bits and federated systems
> eg using Numascale's NumaConnect/NumaChip can have more than 255 memory
> controllers, use 16-bits to store the ID.
>
> v2: Avoid change to intlv_en variable
>
> Signed-off-by: Daniel J Blueman <[email protected]>
> ---
> drivers/edac/amd64_edac.c | 19 ++++++++++---------
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index 18d404a..28b2005 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -942,7 +942,8 @@ static u64 get_error_address(struct mce *m)
> struct amd64_pvt *pvt;
> u64 cc6_base, tmp_addr;
> u32 tmp;
> - u8 mce_nid, intlv_en;
> + u16 mce_nid;
> + u8 intlv_en;
>
> if ((addr & GENMASK(24, 47)) >> 24 != 0x00fdf7)
> return addr;
> @@ -1499,7 +1500,7 @@ static int f1x_match_to_this_node(struct amd64_pvt *pvt, unsigned range,
> u8 channel;
> bool high_range = false;
>
> - u8 node_id = dram_dst_node(pvt, range);
> + u16 node_id = dram_dst_node(pvt, range);

No need for that change since the field dram_dst_node returns is 3 bits
wide (see F1x4{0,4}).

--
Regards/Gruss,
Boris.

2012-10-31 19:38:51

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 3/4] AMD64 EDAC: Cleanup type usage to be consistent

On Wed, Oct 31, 2012 at 01:55:31PM +0800, Daniel J Blueman wrote:
> As the Northbridge IDs are at most 16-bits, use the same type
> consistently.
>
> Signed-off-by: Daniel J Blueman <[email protected]>
> ---
> arch/x86/include/asm/amd_nb.h | 2 +-
> arch/x86/include/asm/processor.h | 2 +-
> arch/x86/kernel/cpu/amd.c | 4 ++--
> drivers/edac/amd64_edac.c | 26 ++++++++++++++------------
> drivers/edac/amd64_edac.h | 2 +-
> 5 files changed, 19 insertions(+), 17 deletions(-)
>
> diff --git a/arch/x86/include/asm/amd_nb.h b/arch/x86/include/asm/amd_nb.h
> index b88fc7a..0cc1045 100644
> --- a/arch/x86/include/asm/amd_nb.h
> +++ b/arch/x86/include/asm/amd_nb.h
> @@ -76,7 +76,7 @@ static inline bool amd_nb_has_feature(unsigned feature)
> return ((amd_northbridges.flags & feature) == feature);
> }
>
> -static inline struct amd_northbridge *node_to_amd_nb(int node)
> +static inline struct amd_northbridge *node_to_amd_nb(u16 node)
> {
> return (node < amd_northbridges.num) ? &amd_northbridges.nb[node] : NULL;
> }
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index ad1fc85..eb3ba58 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -934,7 +934,7 @@ extern void start_thread(struct pt_regs *regs, unsigned long new_ip,
> extern int get_tsc_mode(unsigned long adr);
> extern int set_tsc_mode(unsigned int val);
>
> -extern int amd_get_nb_id(int cpu);
> +extern u16 amd_get_nb_id(int cpu);
>
> struct aperfmperf {
> u64 aperf, mperf;
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index f7e98a2..52cab1f 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -364,9 +364,9 @@ static void __cpuinit amd_detect_cmp(struct cpuinfo_x86 *c)
> #endif
> }
>
> -int amd_get_nb_id(int cpu)
> +u16 amd_get_nb_id(int cpu)
> {
> - int id = 0;
> + u16 id = 0;
> #ifdef CONFIG_SMP
> id = per_cpu(cpu_llc_id, cpu);
> #endif
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index 9920dfd..12cd675 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -239,7 +239,7 @@ static int amd64_get_scrub_rate(struct mem_ctl_info *mci)
> * DRAM base/limit associated with node_id
> */
> static bool amd64_base_limit_match(struct amd64_pvt *pvt, u64 sys_addr,
> - unsigned nid)
> + u16 nid)

This one is used as an index into an 8 elements array, i.e. the DRAM
ranges. I don't think you need u16 for it.

> {
> u64 addr;
>
> @@ -265,7 +265,7 @@ static struct mem_ctl_info *find_mc_by_sys_addr(struct mem_ctl_info *mci,
> u64 sys_addr)
> {
> struct amd64_pvt *pvt;
> - unsigned node_id;
> + u16 node_id;

This is the loop variable over the DRAM_RANGES, so ditto.

> u32 intlv_en, bits;
>
> /*
> @@ -613,7 +613,8 @@ static u64 sys_addr_to_input_addr(struct mem_ctl_info *mci, u64 sys_addr)
> static u64 input_addr_to_dram_addr(struct mem_ctl_info *mci, u64 input_addr)
> {
> struct amd64_pvt *pvt;
> - unsigned node_id, intlv_shift;
> + u16 node_id;

Ok, this takes pvt->mc_node_id so definitely needed.

HOWEVER(!), there's a

BUG_ON(node_id > 7);

on the next line. And the reason you don't hit it is
because this is dead code - its user got removed by
5e2af0c09e60d11dd8297e259a9ca2b3d92d2cf4.

So, you can drop the change to input_addr_to_dram_addr - the whole
function and its caller input_addr_to_sys_addr have to go. I'll remove
them.

> + unsigned intlv_shift;
> u64 bits, dram_addr;
> u32 intlv_sel;
>
> @@ -1337,7 +1338,7 @@ static u8 f1x_determine_channel(struct amd64_pvt *pvt, u64 sys_addr,
> }
>
> /* Convert the sys_addr to the normalized DCT address */
> -static u64 f1x_get_norm_dct_addr(struct amd64_pvt *pvt, unsigned range,
> +static u64 f1x_get_norm_dct_addr(struct amd64_pvt *pvt, u16 range,
> u64 sys_addr, bool hi_rng,
> u32 dct_sel_base_addr)
> {

ditto.

> @@ -1413,7 +1414,7 @@ static int f10_process_possible_spare(struct amd64_pvt *pvt, u8 dct, int csrow)
> * -EINVAL: NOT FOUND
> * 0..csrow = Chip-Select Row
> */
> -static int f1x_lookup_addr_in_dct(u64 in_addr, u32 nid, u8 dct)
> +static int f1x_lookup_addr_in_dct(u64 in_addr, u16 nid, u8 dct)
> {
> struct mem_ctl_info *mci;
> struct amd64_pvt *pvt;
> @@ -1491,7 +1492,7 @@ static u64 f1x_swap_interleaved_region(struct amd64_pvt *pvt, u64 sys_addr)
>
> /* For a given @dram_range, check if @sys_addr falls within it. */
> static int f1x_match_to_this_node(struct amd64_pvt *pvt, unsigned range,
> - u64 sys_addr, int *nid, int *chan_sel)
> + u64 sys_addr, u16 *nid, int *chan_sel)

from staring at this code, we're actually not using that *nid anymore in
f1x_map_sysaddr_to_csrow. And to be honest, it has been like that since
we upstreamed the driver.

So you can drop that change too, I'll audit it more thoroughly and
remove that function argument too.

> {
> int cs_found = -EINVAL;
> u64 chan_addr;
> @@ -1572,10 +1573,10 @@ static int f1x_match_to_this_node(struct amd64_pvt *pvt, unsigned range,
> }
>
> static int f1x_translate_sysaddr_to_cs(struct amd64_pvt *pvt, u64 sys_addr,
> - int *node, int *chan_sel)
> + u16 *node, int *chan_sel)
> {
> int cs_found = -EINVAL;
> - unsigned range;
> + u16 range;
>
> for (range = 0; range < DRAM_RANGES; range++) {
>

ditto.

> @@ -1607,7 +1608,8 @@ static void f1x_map_sysaddr_to_csrow(struct mem_ctl_info *mci, u64 sys_addr,
> {
> struct amd64_pvt *pvt = mci->pvt_info;
> u32 page, offset;
> - int nid, csrow, chan = 0;
> + int csrow, chan = 0;
> + u16 nid;

ditto.

>
> error_address_to_page_and_offset(sys_addr, &page, &offset);
>
> @@ -2065,7 +2067,7 @@ static void read_mc_regs(struct amd64_pvt *pvt)
> struct cpuinfo_x86 *c = &boot_cpu_data;
> u64 msr_val;
> u32 tmp;
> - unsigned range;
> + u16 range;

no need, only iterating over the 8 DRAM ranges.

>
> /*
> * Retrieve TOP_MEM and TOP_MEM2; no masking off of reserved bits since
> @@ -2263,7 +2265,7 @@ static int init_csrows(struct mem_ctl_info *mci)
> }
>
> /* get all cores on this DCT */
> -static void get_cpus_on_this_dct_cpumask(struct cpumask *mask, unsigned nid)
> +static void get_cpus_on_this_dct_cpumask(struct cpumask *mask, u16 nid)
> {
> int cpu;
>
> @@ -2273,7 +2275,7 @@ static void get_cpus_on_this_dct_cpumask(struct cpumask *mask, unsigned nid)
> }
>
> /* check MCG_CTL on all the cpus on this node */
> -static bool amd64_nb_mce_bank_enabled_on_node(unsigned nid)
> +static bool amd64_nb_mce_bank_enabled_on_node(u16 nid)
> {
> cpumask_var_t mask;
> int cpu, nbe;
> diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
> index 90cae61..647c9b8 100644
> --- a/drivers/edac/amd64_edac.h
> +++ b/drivers/edac/amd64_edac.h
> @@ -332,7 +332,7 @@ struct amd64_pvt {
> /* pci_device handles which we utilize */
> struct pci_dev *F1, *F2, *F3;
>
> - unsigned mc_node_id; /* MC index of this MC node */
> + u16 mc_node_id; /* MC index of this MC node */
> int ext_model; /* extended model value of this node */
> int channel_count;
>
> --
> 1.7.9.5
>
>

--
Regards/Gruss,
Boris.

2012-10-31 19:39:52

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 4/4] AMD64 EDAC: Use appropriate name for NB indexing

On Wed, Oct 31, 2012 at 01:55:32PM +0800, Daniel J Blueman wrote:
> Use the same 'amd' prefix as related functions for clarity.
>
> Signed-off-by: Daniel J Blueman <[email protected]>
> ---
> arch/x86/include/asm/amd_nb.h | 2 +-
> drivers/edac/amd64_edac.c | 6 +++---
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/include/asm/amd_nb.h b/arch/x86/include/asm/amd_nb.h
> index 0cc1045..39b5ddd 100644
> --- a/arch/x86/include/asm/amd_nb.h
> +++ b/arch/x86/include/asm/amd_nb.h
> @@ -81,7 +81,7 @@ static inline struct amd_northbridge *node_to_amd_nb(u16 node)
> return (node < amd_northbridges.num) ? &amd_northbridges.nb[node] : NULL;
> }
>
> -static inline u16 get_node_id(struct pci_dev *pdev)
> +static inline u16 amd_get_node_id(struct pci_dev *pdev)
> {
> int i;
>
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index 12cd675..59658b9 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -2558,7 +2558,7 @@ static int amd64_init_one_instance(struct pci_dev *F2)
> struct mem_ctl_info *mci = NULL;
> struct edac_mc_layer layers[2];
> int err = 0, ret;
> - u16 nid = get_node_id(F2);
> + u16 nid = amd_get_node_id(F2);
>
> ret = -ENOMEM;
> pvt = kzalloc(sizeof(struct amd64_pvt), GFP_KERNEL);
> @@ -2649,7 +2649,7 @@ err_ret:
> static int __devinit amd64_probe_one_instance(struct pci_dev *pdev,
> const struct pci_device_id *mc_type)
> {
> - u16 nid = get_node_id(pdev);
> + u16 nid = amd_get_node_id(pdev);
> struct pci_dev *F3 = node_to_amd_nb(nid)->misc;
> struct ecc_settings *s;
> int ret = 0;
> @@ -2699,7 +2699,7 @@ static void __devexit amd64_remove_one_instance(struct pci_dev *pdev)
> {
> struct mem_ctl_info *mci;
> struct amd64_pvt *pvt;
> - u16 nid = get_node_id(pdev);
> + u16 nid = amd_get_node_id(pdev);
> struct pci_dev *F3 = node_to_amd_nb(nid)->misc;
> struct ecc_settings *s = ecc_stngs[nid];

Yeah, you can fold that change into 1/4, actually.

Thanks.

--
Regards/Gruss,
Boris.