* [PATCH] amd64_edac - Add Family 17h Model 70h PCI IDs @ 2019-09-04 21:13 Isaac Vaughn 2019-09-04 21:53 ` Borislav Petkov 0 siblings, 1 reply; 18+ messages in thread From: Isaac Vaughn @ 2019-09-04 21:13 UTC (permalink / raw) To: Borislav Petkov; +Cc: trivial, linux-edac Dear Mr. Petkov, I noticed the current EDAC driver doesn't support the new Zen 2 (model 70h) processors, so I patched the new device ids in. The changes are minimal, I merely extended the existing enums with information from the new models. This is my first kernel patch and I'm not sure I followed the guide (kernel.org/doc/html/v5.2/process/submitting-patches.html) correctly, so please let me know if I need to resubmit this email with updated formatting prior to review. Sincerely, Isaac Vaughn Signed-off-by: Isaac Vaughn <isaac.vaughn@knights.ucf.edu> --- diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c index 873437be86d9..a35c97f9100a 100644 --- a/drivers/edac/amd64_edac.c +++ b/drivers/edac/amd64_edac.c @@ -2253,6 +2253,15 @@ static struct amd64_family_type family_types[] = { .dbam_to_cs = f17_base_addr_to_cs_size, } }, + [F17_M70H_CPUS] = { + .ctl_name = F17h_M70h, + .f0_id = PCI_DEVICE_ID_AMD_17H_M70H_DF_F0, + .f6_id = PCI_DEVICE_ID_AMD_17H_M70H_DF_F6, + .ops = { + .early_channel_count = f17_early_channel_count, + .dbam_to_cs = f17_base_addr_to_cs_size, + } + }, }; /* @@ -3241,6 +3250,10 @@ static struct amd64_family_type *per_family_init(struct amd64_pvt *pvt) fam_type = &family_types[F17_M30H_CPUS]; pvt->ops = &family_types[F17_M30H_CPUS].ops; break; + } else if (pvt->model >= 0x70 && pvt->model <= 0x7f) { + fam_type = &family_types[F17_M70H_CPUS]; + pvt->ops = &family_types[F17_M70H_CPUS].ops; + break; } /* fall through */ case 0x18: diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h index 8f66472f7adc..1adf7ddbf744 100644 --- a/drivers/edac/amd64_edac.h +++ b/drivers/edac/amd64_edac.h @@ -119,6 +119,8 @@ #define PCI_DEVICE_ID_AMD_17H_M10H_DF_F6 0x15ee #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F0 0x1490 #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F6 0x1496 +#define PCI_DEVICE_ID_AMD_17H_M70H_DF_F0 0x1440 +#define PCI_DEVICE_ID_AMD_17H_M70H_DF_F6 0x1446 /* * Function 1 - Address Map @@ -285,6 +287,7 @@ enum amd_families { F17_CPUS, F17_M10H_CPUS, F17_M30H_CPUS, + F17_M70H_CPUS, NUM_FAMILIES, }; ~ ~ ~ ~ ~ ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] amd64_edac - Add Family 17h Model 70h PCI IDs 2019-09-04 21:13 [PATCH] amd64_edac - Add Family 17h Model 70h PCI IDs Isaac Vaughn @ 2019-09-04 21:53 ` Borislav Petkov 2019-09-04 22:21 ` Ghannam, Yazen 2019-09-05 1:21 ` Isaac Vaughn 0 siblings, 2 replies; 18+ messages in thread From: Borislav Petkov @ 2019-09-04 21:53 UTC (permalink / raw) To: Isaac Vaughn, Yazen Ghannam; +Cc: trivial, linux-edac Hi Isaac, On Wed, Sep 04, 2019 at 09:13:47PM +0000, Isaac Vaughn wrote: > I noticed the current EDAC driver doesn't support the new Zen 2 (model > 70h) processors, so I patched the new device ids in. The changes are > minimal, I merely extended the existing enums with information from > the new models. Makes sense to me. @Yazen, ACK? > This is my first kernel patch and I'm not sure I followed the guide > (kernel.org/doc/html/v5.2/process/submitting-patches.html) correctly, > so please let me know if I need to resubmit this email with updated > formatting prior to review. Sure, you're almost there but it needs to have a title and a commit message. If you need an example, look at how a similar patch to yours is formatted: 6e846239e548 ("EDAC/amd64: Add Family 17h Model 30h PCI IDs") Here's a web URL too in case you don't know how to work the git thing :) https://git.kernel.org/linus/6e846239e5487cbb89ac8192d5f11437d010130e That is, if you want to learn how to do patches at all. Otherwise, I can fix it up for ya. Leaving in the rest for Yazen. > Signed-off-by: Isaac Vaughn <isaac.vaughn@knights.ucf.edu> > --- > > diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c > index 873437be86d9..a35c97f9100a 100644 > --- a/drivers/edac/amd64_edac.c > +++ b/drivers/edac/amd64_edac.c > @@ -2253,6 +2253,15 @@ static struct amd64_family_type family_types[] = { > .dbam_to_cs = f17_base_addr_to_cs_size, > } > }, > + [F17_M70H_CPUS] = { > + .ctl_name = F17h_M70h, > + .f0_id = PCI_DEVICE_ID_AMD_17H_M70H_DF_F0, > + .f6_id = PCI_DEVICE_ID_AMD_17H_M70H_DF_F6, > + .ops = { > + .early_channel_count = f17_early_channel_count, > + .dbam_to_cs = f17_base_addr_to_cs_size, > + } > + }, You need to fix the tabbing here to look like the other struct assignments above. > }; > > /* > @@ -3241,6 +3250,10 @@ static struct amd64_family_type *per_family_init(struct amd64_pvt *pvt) > fam_type = &family_types[F17_M30H_CPUS]; > pvt->ops = &family_types[F17_M30H_CPUS].ops; > break; > + } else if (pvt->model >= 0x70 && pvt->model <= 0x7f) { > + fam_type = &family_types[F17_M70H_CPUS]; > + pvt->ops = &family_types[F17_M70H_CPUS].ops; > + break; > } Here too. Your "else if" needs to have the same indentation as the one above. > /* fall through */ > case 0x18: > diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h > index 8f66472f7adc..1adf7ddbf744 100644 > --- a/drivers/edac/amd64_edac.h > +++ b/drivers/edac/amd64_edac.h > @@ -119,6 +119,8 @@ > #define PCI_DEVICE_ID_AMD_17H_M10H_DF_F6 0x15ee > #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F0 0x1490 > #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F6 0x1496 > +#define PCI_DEVICE_ID_AMD_17H_M70H_DF_F0 0x1440 > +#define PCI_DEVICE_ID_AMD_17H_M70H_DF_F6 0x1446 > > /* > * Function 1 - Address Map > @@ -285,6 +287,7 @@ enum amd_families { > F17_CPUS, > F17_M10H_CPUS, > F17_M30H_CPUS, > + F17_M70H_CPUS, And that too. Depending on the text editor, you can show the whitespace characters on each line and then make sure they're the same as on the lines above it. For example with vim you do ":set list" and that would show you: ^I^I} else if (pvt->model >= 0x30 && pvt->model <= 0x3f) {$ ^I^I^Ifam_type = &family_types[F17_M30H_CPUS];$ ^I^I^Ipvt->ops = &family_types[F17_M30H_CPUS].ops;$ ^I^I^Ibreak;$ } else if (pvt->model >= 0x70 && pvt->model <= 0x7f) {$ fam_type = &family_types[F17_M70H_CPUS];$ pvt->ops = &family_types[F17_M70H_CPUS].ops;$ break;$ ^I^I}$ and you can immediately see the difference. We use tabs in the kernel, which appear here as ^I and you have spaces, thus the difference in indentation. There's a checker script in the kernel source directory you can use: $ ./scripts/checkpatch.pl /tmp/isaac.01 WARNING: please, no spaces at the start of a line #34: FILE: drivers/edac/amd64_edac.c:2256: + [F17_M70H_CPUS] = {$ ERROR: code indent should use tabs where possible #35: FILE: drivers/edac/amd64_edac.c:2257: + .ctl_name = F17h_M70h,$ WARNING: please, no spaces at the start of a line #35: FILE: drivers/edac/amd64_edac.c:2257: + .ctl_name = F17h_M70h,$ ERROR: code indent should use tabs where possible #36: FILE: drivers/edac/amd64_edac.c:2258: + .f0_id = PCI_DEVICE_ID_AMD_17H_M70H_DF_F0,$ ... which would show you what is wrong with the patch. And finally, here some more links which you might find useful: https://www.kernel.org/doc/html/v5.2/process/coding-style.html https://kernelnewbies.org/ That is, if you want to get involved in doing the patches thing :-) Thx and have fun! -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH] amd64_edac - Add Family 17h Model 70h PCI IDs 2019-09-04 21:53 ` Borislav Petkov @ 2019-09-04 22:21 ` Ghannam, Yazen 2019-09-05 1:21 ` Isaac Vaughn 1 sibling, 0 replies; 18+ messages in thread From: Ghannam, Yazen @ 2019-09-04 22:21 UTC (permalink / raw) To: Borislav Petkov, Isaac Vaughn; +Cc: trivial, linux-edac > -----Original Message----- > From: linux-edac-owner@vger.kernel.org <linux-edac-owner@vger.kernel.org> On Behalf Of Borislav Petkov > Sent: Wednesday, September 4, 2019 4:53 PM > To: Isaac Vaughn <isaac.vaughn@Knights.ucf.edu>; Ghannam, Yazen <Yazen.Ghannam@amd.com> > Cc: trivial@kernel.org; linux-edac@vger.kernel.org > Subject: Re: [PATCH] amd64_edac - Add Family 17h Model 70h PCI IDs > > Hi Isaac, > > On Wed, Sep 04, 2019 at 09:13:47PM +0000, Isaac Vaughn wrote: > > I noticed the current EDAC driver doesn't support the new Zen 2 (model > > 70h) processors, so I patched the new device ids in. The changes are > > minimal, I merely extended the existing enums with information from > > the new models. > > Makes sense to me. > > @Yazen, ACK? The new PCI IDs are correct. However, we also need Device 18h Function 3 for amd64_edac_mod to function. I don't see a commit upstream that adds this. Isaac, Could you please add the Function 3 as well? Please see the following for reference. be3518a16ef2 ("x86/amd_nb: Add PCI device IDs for family 17h, model 30h") Thank you! -Yazen ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] amd64_edac - Add Family 17h Model 70h PCI IDs 2019-09-04 21:53 ` Borislav Petkov 2019-09-04 22:21 ` Ghannam, Yazen @ 2019-09-05 1:21 ` Isaac Vaughn 2019-09-05 7:09 ` Borislav Petkov 1 sibling, 1 reply; 18+ messages in thread From: Isaac Vaughn @ 2019-09-05 1:21 UTC (permalink / raw) To: Borislav Petkov, Yazen Ghannam; +Cc: trivial, linux-edac Add the new Family 17h Model 70h PCI IDs (device 18h functions 0, 3, 4, and 6) to the kernel, the hwmon module, and the AMD64 EDAC module. Signed-off-by: Isaac Vaughn <isaac.vaughn@knights.ucf.edu> --- Changes to the amd64_edac were tested on 5.2.11 and the current patch was tested on 5.3.0rc7. Since git's pager has been mangling my indentation, diffs were generated with the '--no-pager' option. (Not sure if this might make output non-standard in some other way.) arch/x86/kernel/amd_nb.c | 5 +++++ drivers/edac/amd64_edac.c | 13 +++++++++++++ drivers/edac/amd64_edac.h | 3 +++ drivers/hwmon/k10temp.c | 1 + include/linux/pci_ids.h | 1 + 5 files changed, 23 insertions(+) diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c index d63e63b7d1d9..08cc057126ba 100644 --- a/arch/x86/kernel/amd_nb.c +++ b/arch/x86/kernel/amd_nb.c @@ -18,9 +18,11 @@ #define PCI_DEVICE_ID_AMD_17H_ROOT 0x1450 #define PCI_DEVICE_ID_AMD_17H_M10H_ROOT 0x15d0 #define PCI_DEVICE_ID_AMD_17H_M30H_ROOT 0x1480 +#define PCI_DEVICE_ID_AMD_17H_M70H_ROOT 0x1480 #define PCI_DEVICE_ID_AMD_17H_DF_F4 0x1464 #define PCI_DEVICE_ID_AMD_17H_M10H_DF_F4 0x15ec #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F4 0x1494 +#define PCI_DEVICE_ID_AMD_17H_M70H_DF_F4 0x1444 /* Protect the PCI config register pairs used for SMN and DF indirect access. */ static DEFINE_MUTEX(smn_mutex); @@ -31,6 +33,7 @@ static const struct pci_device_id amd_root_ids[] = { { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_ROOT) }, { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M10H_ROOT) }, { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M30H_ROOT) }, + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M70H_ROOT) }, {} }; @@ -49,6 +52,7 @@ const struct pci_device_id amd_nb_misc_ids[] = { { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_DF_F3) }, { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M10H_DF_F3) }, { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M30H_DF_F3) }, + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M70H_DF_F3) }, { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F3) }, {} }; @@ -63,6 +67,7 @@ static const struct pci_device_id amd_nb_link_ids[] = { { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_DF_F4) }, { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M10H_DF_F4) }, { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M30H_DF_F4) }, + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M70H_DF_F4) }, { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F4) }, {} }; diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c index 873437be86d9..a35c97f9100a 100644 --- a/drivers/edac/amd64_edac.c +++ b/drivers/edac/amd64_edac.c @@ -2253,6 +2253,15 @@ static struct amd64_family_type family_types[] = { .dbam_to_cs = f17_base_addr_to_cs_size, } }, + [F17_M70H_CPUS] = { + .ctl_name = "F17h_M70h", + .f0_id = PCI_DEVICE_ID_AMD_17H_M70H_DF_F0, + .f6_id = PCI_DEVICE_ID_AMD_17H_M70H_DF_F6, + .ops = { + .early_channel_count = f17_early_channel_count, + .dbam_to_cs = f17_base_addr_to_cs_size, + } + }, }; /* @@ -3241,6 +3250,10 @@ static struct amd64_family_type *per_family_init(struct amd64_pvt *pvt) fam_type = &family_types[F17_M30H_CPUS]; pvt->ops = &family_types[F17_M30H_CPUS].ops; break; + } else if (pvt->model >= 0x70 && pvt->model <= 0x7f) { + fam_type = &family_types[F17_M70H_CPUS]; + pvt->ops = &family_types[F17_M70H_CPUS].ops; + break; } /* fall through */ case 0x18: diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h index 8f66472f7adc..1adf7ddbf744 100644 --- a/drivers/edac/amd64_edac.h +++ b/drivers/edac/amd64_edac.h @@ -119,6 +119,8 @@ #define PCI_DEVICE_ID_AMD_17H_M10H_DF_F6 0x15ee #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F0 0x1490 #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F6 0x1496 +#define PCI_DEVICE_ID_AMD_17H_M70H_DF_F0 0x1440 +#define PCI_DEVICE_ID_AMD_17H_M70H_DF_F6 0x1446 /* * Function 1 - Address Map @@ -285,6 +287,7 @@ enum amd_families { F17_CPUS, F17_M10H_CPUS, F17_M30H_CPUS, + F17_M70H_CPUS, NUM_FAMILIES, }; diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c index c77e89239dcd..5c1dddde193c 100644 --- a/drivers/hwmon/k10temp.c +++ b/drivers/hwmon/k10temp.c @@ -349,6 +349,7 @@ static const struct pci_device_id k10temp_id_table[] = { { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_DF_F3) }, { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M10H_DF_F3) }, { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M30H_DF_F3) }, + { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M70H_DF_F3) }, { PCI_VDEVICE(HYGON, PCI_DEVICE_ID_AMD_17H_DF_F3) }, {} }; diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h index c842735a4f45..4b97f427cc92 100644 --- a/include/linux/pci_ids.h +++ b/include/linux/pci_ids.h @@ -548,6 +548,7 @@ #define PCI_DEVICE_ID_AMD_17H_DF_F3 0x1463 #define PCI_DEVICE_ID_AMD_17H_M10H_DF_F3 0x15eb #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F3 0x1493 +#define PCI_DEVICE_ID_AMD_17H_M70H_DF_F3 0x1443 #define PCI_DEVICE_ID_AMD_CNB17H_F3 0x1703 #define PCI_DEVICE_ID_AMD_LANCE 0x2000 #define PCI_DEVICE_ID_AMD_LANCE_HOME 0x2001 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] amd64_edac - Add Family 17h Model 70h PCI IDs 2019-09-05 1:21 ` Isaac Vaughn @ 2019-09-05 7:09 ` Borislav Petkov 2019-09-05 13:17 ` Isaac Vaughn 0 siblings, 1 reply; 18+ messages in thread From: Borislav Petkov @ 2019-09-05 7:09 UTC (permalink / raw) To: Isaac Vaughn; +Cc: Yazen Ghannam, trivial, linux-edac On Thu, Sep 05, 2019 at 01:21:40AM +0000, Isaac Vaughn wrote: > Add the new Family 17h Model 70h PCI IDs (device 18h functions 0, 3, 4, and 6) to the kernel, the hwmon module, and the AMD64 EDAC module. > > Signed-off-by: Isaac Vaughn <isaac.vaughn@knights.ucf.edu> > --- > Changes to the amd64_edac were tested on 5.2.11 and the current patch was tested on 5.3.0rc7. Since git's pager has been mangling my indentation, diffs were generated with the '--no-pager' option. (Not sure if this might make output non-standard in some other way.) > That's probably because this is not how you generate patches with git. Please have a look at one of the countless tutorials on the web how to do that. The gist of it is, you do: $ git commit -a -s <write commit message, add SOB etc> $ git format-patch -1 -o /tmp/ and send the filename as an email or you use $ git send-email -1 ... depending on what you would like to do. Just note that git send-email can spam people quickly so I'd suggest using the --dry-run option there first. :) The tutorials and the manpages of the respective commands will give you more detail about what they all do and what options to use. But before you do that, you need to fix your indentation because that still is wrong. Use that script checkpatch.pl to verify your patch and send it only then when it doesn't complain about spaces anymore, see below. Oh, and pls add those PCI IDs to drivers/edac/amd64_edac.h, not to pci_ids.h because they will be used by the amd64_edac driver only for now so they don't need to go into the system-wide header. Thanks! $ ./scripts/checkpatch.pl /tmp/isaac.vaughn.02 WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line) #14: Add the new Family 17h Model 70h PCI IDs (device 18h functions 0, 3, 4, and 6) to the kernel, the hwmon module, and the AMD64 EDAC module. WARNING: please, no spaces at the start of a line #47: FILE: arch/x86/kernel/amd_nb.c:36: + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M70H_ROOT) },$ WARNING: please, no spaces at the start of a line #55: FILE: arch/x86/kernel/amd_nb.c:55: + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M70H_DF_F3) },$ WARNING: please, no spaces at the start of a line #63: FILE: arch/x86/kernel/amd_nb.c:70: + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M70H_DF_F4) },$ WARNING: please, no spaces at the start of a line #75: FILE: drivers/edac/amd64_edac.c:2256: + [F17_M70H_CPUS] = {$ ... -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] amd64_edac - Add Family 17h Model 70h PCI IDs 2019-09-05 7:09 ` Borislav Petkov @ 2019-09-05 13:17 ` Isaac Vaughn 2019-09-05 13:54 ` Borislav Petkov 0 siblings, 1 reply; 18+ messages in thread From: Isaac Vaughn @ 2019-09-05 13:17 UTC (permalink / raw) To: Borislav Petkov; +Cc: Yazen Ghannam, trivial, linux-edac > Oh, and pls add those PCI IDs to drivers/edac/amd64_edac.h, not to > pci_ids.h because they will be used by the amd64_edac driver only for > now so they don't need to go into the system-wide header. Does this apply to all the PCI IDs? Functions 0 and 6 are in amd64_edac.h, but some of the additional IDs had model 30h equivalents in different places. For the additional IDs Yazen pointed out, I imitated the changes from commit be3518a16ef270e3b030a6ae96055f83f51bd3dd (x86/amd_nb: Add PCI device IDs for family 17h, model 30h). Sincerely, Isaac Vaughn ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] amd64_edac - Add Family 17h Model 70h PCI IDs 2019-09-05 13:17 ` Isaac Vaughn @ 2019-09-05 13:54 ` Borislav Petkov 2019-09-06 1:01 ` [PATCH] Add PCI device IDs for family 17h, model 70h Isaac Vaughn 2019-09-06 1:56 ` Isaac Vaughn 0 siblings, 2 replies; 18+ messages in thread From: Borislav Petkov @ 2019-09-05 13:54 UTC (permalink / raw) To: Isaac Vaughn; +Cc: Yazen Ghannam, trivial, linux-edac On Thu, Sep 05, 2019 at 01:17:46PM +0000, Isaac Vaughn wrote: > > Oh, and pls add those PCI IDs to drivers/edac/amd64_edac.h, not to > > pci_ids.h because they will be used by the amd64_edac driver only for > > now so they don't need to go into the system-wide header. > Does this apply to all the PCI IDs? This applies to PCI IDs which are used by a single driver only. They go in its header. If something else needs them, then they get moved to pci_ids.h. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] Add PCI device IDs for family 17h, model 70h 2019-09-05 13:54 ` Borislav Petkov @ 2019-09-06 1:01 ` Isaac Vaughn 2019-09-06 1:41 ` Ghannam, Yazen 2019-09-06 1:56 ` Isaac Vaughn 1 sibling, 1 reply; 18+ messages in thread From: Isaac Vaughn @ 2019-09-06 1:01 UTC (permalink / raw) To: Borislav Petkov; +Cc: Isaac Vaughn, Yazen Ghannam, trivial, linux-edac Add the new Family 17h Model 70h PCI IDs (device 18h functions 0,3,4,6) to the kernel, the hwmon module, and the AMD64 EDAC module. Signed-off-by: Isaac Vaughn <isaac.vaughn@knights.ucf.edu> --- arch/x86/kernel/amd_nb.c | 5 +++++ drivers/edac/amd64_edac.c | 13 +++++++++++++ drivers/edac/amd64_edac.h | 3 +++ drivers/hwmon/k10temp.c | 1 + include/linux/pci_ids.h | 1 + 5 files changed, 23 insertions(+) diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c index d63e63b7d1d9..08cc057126ba 100644 --- a/arch/x86/kernel/amd_nb.c +++ b/arch/x86/kernel/amd_nb.c @@ -18,9 +18,11 @@ #define PCI_DEVICE_ID_AMD_17H_ROOT 0x1450 #define PCI_DEVICE_ID_AMD_17H_M10H_ROOT 0x15d0 #define PCI_DEVICE_ID_AMD_17H_M30H_ROOT 0x1480 +#define PCI_DEVICE_ID_AMD_17H_M70H_ROOT 0x1480 #define PCI_DEVICE_ID_AMD_17H_DF_F4 0x1464 #define PCI_DEVICE_ID_AMD_17H_M10H_DF_F4 0x15ec #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F4 0x1494 +#define PCI_DEVICE_ID_AMD_17H_M70H_DF_F4 0x1444 /* Protect the PCI config register pairs used for SMN and DF indirect access. */ static DEFINE_MUTEX(smn_mutex); @@ -31,6 +33,7 @@ static const struct pci_device_id amd_root_ids[] = { { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_ROOT) }, { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M10H_ROOT) }, { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M30H_ROOT) }, + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M70H_ROOT) }, {} }; @@ -49,6 +52,7 @@ const struct pci_device_id amd_nb_misc_ids[] = { { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_DF_F3) }, { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M10H_DF_F3) }, { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M30H_DF_F3) }, + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M70H_DF_F3) }, { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F3) }, {} }; @@ -63,6 +67,7 @@ static const struct pci_device_id amd_nb_link_ids[] = { { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_DF_F4) }, { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M10H_DF_F4) }, { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M30H_DF_F4) }, + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M70H_DF_F4) }, { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F4) }, {} }; diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c index 873437be86d9..a35c97f9100a 100644 --- a/drivers/edac/amd64_edac.c +++ b/drivers/edac/amd64_edac.c @@ -2253,6 +2253,15 @@ static struct amd64_family_type family_types[] = { .dbam_to_cs = f17_base_addr_to_cs_size, } }, + [F17_M70H_CPUS] = { + .ctl_name = "F17h_M70h", + .f0_id = PCI_DEVICE_ID_AMD_17H_M70H_DF_F0, + .f6_id = PCI_DEVICE_ID_AMD_17H_M70H_DF_F6, + .ops = { + .early_channel_count = f17_early_channel_count, + .dbam_to_cs = f17_base_addr_to_cs_size, + } + }, }; /* @@ -3241,6 +3250,10 @@ static struct amd64_family_type *per_family_init(struct amd64_pvt *pvt) fam_type = &family_types[F17_M30H_CPUS]; pvt->ops = &family_types[F17_M30H_CPUS].ops; break; + } else if (pvt->model >= 0x70 && pvt->model <= 0x7f) { + fam_type = &family_types[F17_M70H_CPUS]; + pvt->ops = &family_types[F17_M70H_CPUS].ops; + break; } /* fall through */ case 0x18: diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h index 8f66472f7adc..1adf7ddbf744 100644 --- a/drivers/edac/amd64_edac.h +++ b/drivers/edac/amd64_edac.h @@ -119,6 +119,8 @@ #define PCI_DEVICE_ID_AMD_17H_M10H_DF_F6 0x15ee #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F0 0x1490 #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F6 0x1496 +#define PCI_DEVICE_ID_AMD_17H_M70H_DF_F0 0x1440 +#define PCI_DEVICE_ID_AMD_17H_M70H_DF_F6 0x1446 /* * Function 1 - Address Map @@ -285,6 +287,7 @@ enum amd_families { F17_CPUS, F17_M10H_CPUS, F17_M30H_CPUS, + F17_M70H_CPUS, NUM_FAMILIES, }; diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c index c77e89239dcd..5c1dddde193c 100644 --- a/drivers/hwmon/k10temp.c +++ b/drivers/hwmon/k10temp.c @@ -349,6 +349,7 @@ static const struct pci_device_id k10temp_id_table[] = { { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_DF_F3) }, { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M10H_DF_F3) }, { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M30H_DF_F3) }, + { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M70H_DF_F3) }, { PCI_VDEVICE(HYGON, PCI_DEVICE_ID_AMD_17H_DF_F3) }, {} }; diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h index c842735a4f45..4b97f427cc92 100644 --- a/include/linux/pci_ids.h +++ b/include/linux/pci_ids.h @@ -548,6 +548,7 @@ #define PCI_DEVICE_ID_AMD_17H_DF_F3 0x1463 #define PCI_DEVICE_ID_AMD_17H_M10H_DF_F3 0x15eb #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F3 0x1493 +#define PCI_DEVICE_ID_AMD_17H_M70H_DF_F3 0x1443 #define PCI_DEVICE_ID_AMD_CNB17H_F3 0x1703 #define PCI_DEVICE_ID_AMD_LANCE 0x2000 #define PCI_DEVICE_ID_AMD_LANCE_HOME 0x2001 -- 2.23.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* RE: [PATCH] Add PCI device IDs for family 17h, model 70h 2019-09-06 1:01 ` [PATCH] Add PCI device IDs for family 17h, model 70h Isaac Vaughn @ 2019-09-06 1:41 ` Ghannam, Yazen 0 siblings, 0 replies; 18+ messages in thread From: Ghannam, Yazen @ 2019-09-06 1:41 UTC (permalink / raw) To: Isaac Vaughn, Borislav Petkov; +Cc: trivial, linux-edac > -----Original Message----- > From: Isaac Vaughn <isaac.vaughn@Knights.ucf.edu> > Sent: Thursday, September 5, 2019 8:02 PM > To: Borislav Petkov <bp@alien8.de> > Cc: Isaac Vaughn <isaac.vaughn@Knights.ucf.edu>; Ghannam, Yazen <Yazen.Ghannam@amd.com>; trivial@kernel.org; linux- > edac@vger.kernel.org > Subject: [PATCH] Add PCI device IDs for family 17h, model 70h > > Add the new Family 17h Model 70h PCI IDs (device 18h functions 0,3,4,6) > to the kernel, the hwmon module, and the AMD64 EDAC module. > > Signed-off-by: Isaac Vaughn <isaac.vaughn@knights.ucf.edu> > --- > arch/x86/kernel/amd_nb.c | 5 +++++ > drivers/edac/amd64_edac.c | 13 +++++++++++++ > drivers/edac/amd64_edac.h | 3 +++ > drivers/hwmon/k10temp.c | 1 + > include/linux/pci_ids.h | 1 + > 5 files changed, 23 insertions(+) > > diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c > index d63e63b7d1d9..08cc057126ba 100644 > --- a/arch/x86/kernel/amd_nb.c > +++ b/arch/x86/kernel/amd_nb.c > @@ -18,9 +18,11 @@ > #define PCI_DEVICE_ID_AMD_17H_ROOT 0x1450 > #define PCI_DEVICE_ID_AMD_17H_M10H_ROOT 0x15d0 > #define PCI_DEVICE_ID_AMD_17H_M30H_ROOT 0x1480 > +#define PCI_DEVICE_ID_AMD_17H_M70H_ROOT 0x1480 Since this ID is already defined, you don't need to add it again. The code that searches for the PCI device will match on the ID and return a pointer to the device. The name we associate with the ID is arbitrary. By convention, we use the family/model where the ID first appears, but the name itself isn't important. Thanks, Yazen ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] Add PCI device IDs for family 17h, model 70h 2019-09-05 13:54 ` Borislav Petkov 2019-09-06 1:01 ` [PATCH] Add PCI device IDs for family 17h, model 70h Isaac Vaughn @ 2019-09-06 1:56 ` Isaac Vaughn 2019-09-06 9:12 ` Borislav Petkov 1 sibling, 1 reply; 18+ messages in thread From: Isaac Vaughn @ 2019-09-06 1:56 UTC (permalink / raw) To: Borislav Petkov; +Cc: Isaac Vaughn, Yazen Ghannam, trivial, linux-edac Add the new Family 17h Model 70h PCI IDs (device 18h functions 0,3,4,6) to the kernel, the hwmon module, and the AMD64 EDAC module. Signed-off-by: Isaac Vaughn <isaac.vaughn@knights.ucf.edu> --- arch/x86/kernel/amd_nb.c | 3 +++ drivers/edac/amd64_edac.c | 13 +++++++++++++ drivers/edac/amd64_edac.h | 3 +++ drivers/hwmon/k10temp.c | 1 + include/linux/pci_ids.h | 1 + 5 files changed, 21 insertions(+) diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c index d63e63b7d1d9..0a8b816857c1 100644 --- a/arch/x86/kernel/amd_nb.c +++ b/arch/x86/kernel/amd_nb.c @@ -21,6 +21,7 @@ #define PCI_DEVICE_ID_AMD_17H_DF_F4 0x1464 #define PCI_DEVICE_ID_AMD_17H_M10H_DF_F4 0x15ec #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F4 0x1494 +#define PCI_DEVICE_ID_AMD_17H_M70H_DF_F4 0x1444 /* Protect the PCI config register pairs used for SMN and DF indirect access. */ static DEFINE_MUTEX(smn_mutex); @@ -49,6 +50,7 @@ const struct pci_device_id amd_nb_misc_ids[] = { { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_DF_F3) }, { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M10H_DF_F3) }, { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M30H_DF_F3) }, + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M70H_DF_F3) }, { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F3) }, {} }; @@ -63,6 +65,7 @@ static const struct pci_device_id amd_nb_link_ids[] = { { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_DF_F4) }, { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M10H_DF_F4) }, { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M30H_DF_F4) }, + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M70H_DF_F4) }, { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F4) }, {} }; diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c index 873437be86d9..a35c97f9100a 100644 --- a/drivers/edac/amd64_edac.c +++ b/drivers/edac/amd64_edac.c @@ -2253,6 +2253,15 @@ static struct amd64_family_type family_types[] = { .dbam_to_cs = f17_base_addr_to_cs_size, } }, + [F17_M70H_CPUS] = { + .ctl_name = "F17h_M70h", + .f0_id = PCI_DEVICE_ID_AMD_17H_M70H_DF_F0, + .f6_id = PCI_DEVICE_ID_AMD_17H_M70H_DF_F6, + .ops = { + .early_channel_count = f17_early_channel_count, + .dbam_to_cs = f17_base_addr_to_cs_size, + } + }, }; /* @@ -3241,6 +3250,10 @@ static struct amd64_family_type *per_family_init(struct amd64_pvt *pvt) fam_type = &family_types[F17_M30H_CPUS]; pvt->ops = &family_types[F17_M30H_CPUS].ops; break; + } else if (pvt->model >= 0x70 && pvt->model <= 0x7f) { + fam_type = &family_types[F17_M70H_CPUS]; + pvt->ops = &family_types[F17_M70H_CPUS].ops; + break; } /* fall through */ case 0x18: diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h index 8f66472f7adc..1adf7ddbf744 100644 --- a/drivers/edac/amd64_edac.h +++ b/drivers/edac/amd64_edac.h @@ -119,6 +119,8 @@ #define PCI_DEVICE_ID_AMD_17H_M10H_DF_F6 0x15ee #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F0 0x1490 #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F6 0x1496 +#define PCI_DEVICE_ID_AMD_17H_M70H_DF_F0 0x1440 +#define PCI_DEVICE_ID_AMD_17H_M70H_DF_F6 0x1446 /* * Function 1 - Address Map @@ -285,6 +287,7 @@ enum amd_families { F17_CPUS, F17_M10H_CPUS, F17_M30H_CPUS, + F17_M70H_CPUS, NUM_FAMILIES, }; diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c index c77e89239dcd..5c1dddde193c 100644 --- a/drivers/hwmon/k10temp.c +++ b/drivers/hwmon/k10temp.c @@ -349,6 +349,7 @@ static const struct pci_device_id k10temp_id_table[] = { { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_DF_F3) }, { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M10H_DF_F3) }, { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M30H_DF_F3) }, + { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M70H_DF_F3) }, { PCI_VDEVICE(HYGON, PCI_DEVICE_ID_AMD_17H_DF_F3) }, {} }; diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h index c842735a4f45..4b97f427cc92 100644 --- a/include/linux/pci_ids.h +++ b/include/linux/pci_ids.h @@ -548,6 +548,7 @@ #define PCI_DEVICE_ID_AMD_17H_DF_F3 0x1463 #define PCI_DEVICE_ID_AMD_17H_M10H_DF_F3 0x15eb #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F3 0x1493 +#define PCI_DEVICE_ID_AMD_17H_M70H_DF_F3 0x1443 #define PCI_DEVICE_ID_AMD_CNB17H_F3 0x1703 #define PCI_DEVICE_ID_AMD_LANCE 0x2000 #define PCI_DEVICE_ID_AMD_LANCE_HOME 0x2001 -- 2.23.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] Add PCI device IDs for family 17h, model 70h 2019-09-06 1:56 ` Isaac Vaughn @ 2019-09-06 9:12 ` Borislav Petkov [not found] ` <20190906075729.9e2faf7147da62fc26006833@knights.ucf.edu> 2019-09-06 13:02 ` Guenter Roeck 0 siblings, 2 replies; 18+ messages in thread From: Borislav Petkov @ 2019-09-06 9:12 UTC (permalink / raw) To: Isaac Vaughn Cc: Yazen Ghannam, trivial, linux-edac, x86, linux-hwmon, linux-pci On Fri, Sep 06, 2019 at 01:56:56AM +0000, Isaac Vaughn wrote: > Add the new Family 17h Model 70h PCI IDs (device 18h functions 0,3,4,6) > to the kernel, the hwmon module, and the AMD64 EDAC module. > > Signed-off-by: Isaac Vaughn <isaac.vaughn@knights.ucf.edu> > --- > arch/x86/kernel/amd_nb.c | 3 +++ > drivers/edac/amd64_edac.c | 13 +++++++++++++ > drivers/edac/amd64_edac.h | 3 +++ > drivers/hwmon/k10temp.c | 1 + > include/linux/pci_ids.h | 1 + > 5 files changed, 21 insertions(+) Ok, next lessons :) Before you send a patch, do: $ git diff | ./scripts/get_maintainer.pl or if you've committed it already: $ git log -p -1 | ./scripts/get_maintainer.pl to know who to Cc. I've added the respective mailing lists to Cc because this is a trivial addition of PCI IDs and generally maintainers want to see it as an FYI thing, thus Ccing the mailing list only. But you can just as well Cc the maintainers too - they can manage the mail volume. :) And last but *definitely* not least, the most important lesson: *always* - I can't stress this enough - *always* build *and* test your patch before sending: drivers/edac/amd64_edac.c:2326:19: error: ‘f17_base_addr_to_cs_size’ undeclared here (not in a function); did you mean ‘f17_addr_mask_to_cs_size’? .dbam_to_cs = f17_base_addr_to_cs_size, ^~~~~~~~~~~~~~~~~~~~~~~~ f17_addr_mask_to_cs_size make[2]: *** [scripts/Makefile.build:273: drivers/edac/amd64_edac.o] Error 1 make[2]: *** Waiting for unfinished jobs.... make[1]: *** [scripts/Makefile.build:490: drivers/edac] Error 2 make[1]: *** Waiting for unfinished jobs.... make: *** [Makefile:1076: drivers] Error 2 make: *** Waiting for unfinished jobs.... If it is any consolation: we have all done this mistake in a hurry at least once in the past but pls do take your time. Thx. Leaving in the rest for reference for the newly CCed. > diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c > index d63e63b7d1d9..0a8b816857c1 100644 > --- a/arch/x86/kernel/amd_nb.c > +++ b/arch/x86/kernel/amd_nb.c > @@ -21,6 +21,7 @@ > #define PCI_DEVICE_ID_AMD_17H_DF_F4 0x1464 > #define PCI_DEVICE_ID_AMD_17H_M10H_DF_F4 0x15ec > #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F4 0x1494 > +#define PCI_DEVICE_ID_AMD_17H_M70H_DF_F4 0x1444 > > /* Protect the PCI config register pairs used for SMN and DF indirect access. */ > static DEFINE_MUTEX(smn_mutex); > @@ -49,6 +50,7 @@ const struct pci_device_id amd_nb_misc_ids[] = { > { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_DF_F3) }, > { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M10H_DF_F3) }, > { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M30H_DF_F3) }, > + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M70H_DF_F3) }, > { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F3) }, > {} > }; > @@ -63,6 +65,7 @@ static const struct pci_device_id amd_nb_link_ids[] = { > { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_DF_F4) }, > { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M10H_DF_F4) }, > { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M30H_DF_F4) }, > + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M70H_DF_F4) }, > { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F4) }, > {} > }; > diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c > index 873437be86d9..a35c97f9100a 100644 > --- a/drivers/edac/amd64_edac.c > +++ b/drivers/edac/amd64_edac.c > @@ -2253,6 +2253,15 @@ static struct amd64_family_type family_types[] = { > .dbam_to_cs = f17_base_addr_to_cs_size, > } > }, > + [F17_M70H_CPUS] = { > + .ctl_name = "F17h_M70h", > + .f0_id = PCI_DEVICE_ID_AMD_17H_M70H_DF_F0, > + .f6_id = PCI_DEVICE_ID_AMD_17H_M70H_DF_F6, > + .ops = { > + .early_channel_count = f17_early_channel_count, > + .dbam_to_cs = f17_base_addr_to_cs_size, > + } > + }, > }; > > /* > @@ -3241,6 +3250,10 @@ static struct amd64_family_type *per_family_init(struct amd64_pvt *pvt) > fam_type = &family_types[F17_M30H_CPUS]; > pvt->ops = &family_types[F17_M30H_CPUS].ops; > break; > + } else if (pvt->model >= 0x70 && pvt->model <= 0x7f) { > + fam_type = &family_types[F17_M70H_CPUS]; > + pvt->ops = &family_types[F17_M70H_CPUS].ops; > + break; > } > /* fall through */ > case 0x18: > diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h > index 8f66472f7adc..1adf7ddbf744 100644 > --- a/drivers/edac/amd64_edac.h > +++ b/drivers/edac/amd64_edac.h > @@ -119,6 +119,8 @@ > #define PCI_DEVICE_ID_AMD_17H_M10H_DF_F6 0x15ee > #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F0 0x1490 > #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F6 0x1496 > +#define PCI_DEVICE_ID_AMD_17H_M70H_DF_F0 0x1440 > +#define PCI_DEVICE_ID_AMD_17H_M70H_DF_F6 0x1446 > > /* > * Function 1 - Address Map > @@ -285,6 +287,7 @@ enum amd_families { > F17_CPUS, > F17_M10H_CPUS, > F17_M30H_CPUS, > + F17_M70H_CPUS, > NUM_FAMILIES, > }; > > diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c > index c77e89239dcd..5c1dddde193c 100644 > --- a/drivers/hwmon/k10temp.c > +++ b/drivers/hwmon/k10temp.c > @@ -349,6 +349,7 @@ static const struct pci_device_id k10temp_id_table[] = { > { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_DF_F3) }, > { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M10H_DF_F3) }, > { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M30H_DF_F3) }, > + { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M70H_DF_F3) }, > { PCI_VDEVICE(HYGON, PCI_DEVICE_ID_AMD_17H_DF_F3) }, > {} > }; > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h > index c842735a4f45..4b97f427cc92 100644 > --- a/include/linux/pci_ids.h > +++ b/include/linux/pci_ids.h > @@ -548,6 +548,7 @@ > #define PCI_DEVICE_ID_AMD_17H_DF_F3 0x1463 > #define PCI_DEVICE_ID_AMD_17H_M10H_DF_F3 0x15eb > #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F3 0x1493 > +#define PCI_DEVICE_ID_AMD_17H_M70H_DF_F3 0x1443 > #define PCI_DEVICE_ID_AMD_CNB17H_F3 0x1703 > #define PCI_DEVICE_ID_AMD_LANCE 0x2000 > #define PCI_DEVICE_ID_AMD_LANCE_HOME 0x2001 > -- > 2.23.0 -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <20190906075729.9e2faf7147da62fc26006833@knights.ucf.edu>]
* Re: [PATCH] Add PCI device IDs for family 17h, model 70h [not found] ` <20190906075729.9e2faf7147da62fc26006833@knights.ucf.edu> @ 2019-09-06 12:14 ` Borislav Petkov 0 siblings, 0 replies; 18+ messages in thread From: Borislav Petkov @ 2019-09-06 12:14 UTC (permalink / raw) To: Isaac Vaughn Cc: Yazen Ghannam, trivial, linux-edac, x86, linux-hwmon, linux-pci On Fri, Sep 06, 2019 at 11:57:35AM +0000, Isaac Vaughn wrote: > Will do, thanks. And, when you reply to mails, please press "Reply-to-all" in your mail client and do not do private mails. There's a reason there's a Cc list. Readding them. > I built my patch on both 5.3.0rc7 and master, and have tested and > am currently running it on 5.3.0rc7. Which tag are you building > on? I suspect there is a version mismatch since ".dbam_to_cs = > f17_base_addr_to_cs_size" is on line 2262 for me and a recursive grep > of the entire returned no results for "f17_addr_mask_to_cs_size". Ah, yes, you couldn't have known that, lemme explain. So almost all maintainers have one or more branches which contain the work scheduled to go to Linus in the next merge window and that work is being merged on a daily basis into linux-next. The EDAC branch which has that is edac-for-next in the repo git://git.kernel.org/pub/scm/linux/kernel/git/ras/ras.git Please use that one to base your patch on. That branch also has a previous patch which does this: -static int f17_base_addr_to_cs_size(struct amd64_pvt *pvt, u8 umc, +static int f17_addr_mask_to_cs_size(struct amd64_pvt *pvt, u8 umc, which explains the failure. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] Add PCI device IDs for family 17h, model 70h 2019-09-06 9:12 ` Borislav Petkov [not found] ` <20190906075729.9e2faf7147da62fc26006833@knights.ucf.edu> @ 2019-09-06 13:02 ` Guenter Roeck 2019-09-06 13:09 ` Boris Petkov 1 sibling, 1 reply; 18+ messages in thread From: Guenter Roeck @ 2019-09-06 13:02 UTC (permalink / raw) To: Borislav Petkov Cc: Isaac Vaughn, Yazen Ghannam, trivial, linux-edac, x86, linux-hwmon, linux-pci On Fri, Sep 06, 2019 at 11:12:50AM +0200, Borislav Petkov wrote: > On Fri, Sep 06, 2019 at 01:56:56AM +0000, Isaac Vaughn wrote: > > Add the new Family 17h Model 70h PCI IDs (device 18h functions 0,3,4,6) > > to the kernel, the hwmon module, and the AMD64 EDAC module. > > > > Signed-off-by: Isaac Vaughn <isaac.vaughn@knights.ucf.edu> > > --- > > arch/x86/kernel/amd_nb.c | 3 +++ > > drivers/edac/amd64_edac.c | 13 +++++++++++++ > > drivers/edac/amd64_edac.h | 3 +++ > > drivers/hwmon/k10temp.c | 1 + > > include/linux/pci_ids.h | 1 + > > 5 files changed, 21 insertions(+) > > Ok, next lessons :) > > Before you send a patch, do: > Also, how is this patch different to the patches already in linux-next ? Guenter > $ git diff | ./scripts/get_maintainer.pl > > or if you've committed it already: > > $ git log -p -1 | ./scripts/get_maintainer.pl > > to know who to Cc. > > I've added the respective mailing lists to Cc because this is a trivial > addition of PCI IDs and generally maintainers want to see it as an FYI > thing, thus Ccing the mailing list only. But you can just as well Cc the > maintainers too - they can manage the mail volume. :) > > And last but *definitely* not least, the most important lesson: *always* > - I can't stress this enough - *always* build *and* test your patch > before sending: > > drivers/edac/amd64_edac.c:2326:19: error: ‘f17_base_addr_to_cs_size’ undeclared here (not in a function); did you mean ‘f17_addr_mask_to_cs_size’? > .dbam_to_cs = f17_base_addr_to_cs_size, > ^~~~~~~~~~~~~~~~~~~~~~~~ > f17_addr_mask_to_cs_size > make[2]: *** [scripts/Makefile.build:273: drivers/edac/amd64_edac.o] Error 1 > make[2]: *** Waiting for unfinished jobs.... > make[1]: *** [scripts/Makefile.build:490: drivers/edac] Error 2 > make[1]: *** Waiting for unfinished jobs.... > make: *** [Makefile:1076: drivers] Error 2 > make: *** Waiting for unfinished jobs.... > > If it is any consolation: we have all done this mistake in a hurry at > least once in the past but pls do take your time. > > Thx. > > Leaving in the rest for reference for the newly CCed. > > > diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c > > index d63e63b7d1d9..0a8b816857c1 100644 > > --- a/arch/x86/kernel/amd_nb.c > > +++ b/arch/x86/kernel/amd_nb.c > > @@ -21,6 +21,7 @@ > > #define PCI_DEVICE_ID_AMD_17H_DF_F4 0x1464 > > #define PCI_DEVICE_ID_AMD_17H_M10H_DF_F4 0x15ec > > #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F4 0x1494 > > +#define PCI_DEVICE_ID_AMD_17H_M70H_DF_F4 0x1444 > > > > /* Protect the PCI config register pairs used for SMN and DF indirect access. */ > > static DEFINE_MUTEX(smn_mutex); > > @@ -49,6 +50,7 @@ const struct pci_device_id amd_nb_misc_ids[] = { > > { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_DF_F3) }, > > { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M10H_DF_F3) }, > > { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M30H_DF_F3) }, > > + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M70H_DF_F3) }, > > { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F3) }, > > {} > > }; > > @@ -63,6 +65,7 @@ static const struct pci_device_id amd_nb_link_ids[] = { > > { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_DF_F4) }, > > { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M10H_DF_F4) }, > > { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M30H_DF_F4) }, > > + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M70H_DF_F4) }, > > { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F4) }, > > {} > > }; > > diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c > > index 873437be86d9..a35c97f9100a 100644 > > --- a/drivers/edac/amd64_edac.c > > +++ b/drivers/edac/amd64_edac.c > > @@ -2253,6 +2253,15 @@ static struct amd64_family_type family_types[] = { > > .dbam_to_cs = f17_base_addr_to_cs_size, > > } > > }, > > + [F17_M70H_CPUS] = { > > + .ctl_name = "F17h_M70h", > > + .f0_id = PCI_DEVICE_ID_AMD_17H_M70H_DF_F0, > > + .f6_id = PCI_DEVICE_ID_AMD_17H_M70H_DF_F6, > > + .ops = { > > + .early_channel_count = f17_early_channel_count, > > + .dbam_to_cs = f17_base_addr_to_cs_size, > > + } > > + }, > > }; > > > > /* > > @@ -3241,6 +3250,10 @@ static struct amd64_family_type *per_family_init(struct amd64_pvt *pvt) > > fam_type = &family_types[F17_M30H_CPUS]; > > pvt->ops = &family_types[F17_M30H_CPUS].ops; > > break; > > + } else if (pvt->model >= 0x70 && pvt->model <= 0x7f) { > > + fam_type = &family_types[F17_M70H_CPUS]; > > + pvt->ops = &family_types[F17_M70H_CPUS].ops; > > + break; > > } > > /* fall through */ > > case 0x18: > > diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h > > index 8f66472f7adc..1adf7ddbf744 100644 > > --- a/drivers/edac/amd64_edac.h > > +++ b/drivers/edac/amd64_edac.h > > @@ -119,6 +119,8 @@ > > #define PCI_DEVICE_ID_AMD_17H_M10H_DF_F6 0x15ee > > #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F0 0x1490 > > #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F6 0x1496 > > +#define PCI_DEVICE_ID_AMD_17H_M70H_DF_F0 0x1440 > > +#define PCI_DEVICE_ID_AMD_17H_M70H_DF_F6 0x1446 > > > > /* > > * Function 1 - Address Map > > @@ -285,6 +287,7 @@ enum amd_families { > > F17_CPUS, > > F17_M10H_CPUS, > > F17_M30H_CPUS, > > + F17_M70H_CPUS, > > NUM_FAMILIES, > > }; > > > > diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c > > index c77e89239dcd..5c1dddde193c 100644 > > --- a/drivers/hwmon/k10temp.c > > +++ b/drivers/hwmon/k10temp.c > > @@ -349,6 +349,7 @@ static const struct pci_device_id k10temp_id_table[] = { > > { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_DF_F3) }, > > { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M10H_DF_F3) }, > > { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M30H_DF_F3) }, > > + { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M70H_DF_F3) }, > > { PCI_VDEVICE(HYGON, PCI_DEVICE_ID_AMD_17H_DF_F3) }, > > {} > > }; > > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h > > index c842735a4f45..4b97f427cc92 100644 > > --- a/include/linux/pci_ids.h > > +++ b/include/linux/pci_ids.h > > @@ -548,6 +548,7 @@ > > #define PCI_DEVICE_ID_AMD_17H_DF_F3 0x1463 > > #define PCI_DEVICE_ID_AMD_17H_M10H_DF_F3 0x15eb > > #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F3 0x1493 > > +#define PCI_DEVICE_ID_AMD_17H_M70H_DF_F3 0x1443 > > #define PCI_DEVICE_ID_AMD_CNB17H_F3 0x1703 > > #define PCI_DEVICE_ID_AMD_LANCE 0x2000 > > #define PCI_DEVICE_ID_AMD_LANCE_HOME 0x2001 > > -- > > 2.23.0 > > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] Add PCI device IDs for family 17h, model 70h 2019-09-06 13:02 ` Guenter Roeck @ 2019-09-06 13:09 ` Boris Petkov [not found] ` <B08C8E54-43FA-4E29-8D7D-5F9C4AF20CCF@Knights.ucf.edu> 2019-09-06 16:11 ` Guenter Roeck 0 siblings, 2 replies; 18+ messages in thread From: Boris Petkov @ 2019-09-06 13:09 UTC (permalink / raw) To: Guenter Roeck Cc: Isaac Vaughn, Yazen Ghannam, trivial, linux-edac, x86, linux-hwmon, linux-pci On September 6, 2019 3:02:06 PM GMT+02:00, Guenter Roeck <linux@roeck-us.net> wrote: >Also, how is this patch different to the patches already in linux-next >? Which are those? Care to share their linux-next commit IDs with us? -- Sent from a small device: formatting sux and brevity is inevitable. ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <B08C8E54-43FA-4E29-8D7D-5F9C4AF20CCF@Knights.ucf.edu>]
* Re: [PATCH] Add PCI device IDs for family 17h, model 70h [not found] ` <B08C8E54-43FA-4E29-8D7D-5F9C4AF20CCF@Knights.ucf.edu> @ 2019-09-06 14:50 ` Borislav Petkov 2019-09-06 23:27 ` Isaac Vaughn 0 siblings, 1 reply; 18+ messages in thread From: Borislav Petkov @ 2019-09-06 14:50 UTC (permalink / raw) To: Isaac Vaughn Cc: Guenter Roeck, Yazen Ghannam, trivial, linux-edac, x86, linux-hwmon, linux-pci On Fri, Sep 06, 2019 at 02:05:35PM +0000, Isaac Vaughn wrote: > They seem to have already covered the changes to hwmon and amd_nb, so > the difference is probably the changes to amd64_edac. Yeah, then you can send me only those changes against edac-for-next. The driver won't load on Zen2 before the amd_nb.c changes have landed upstream but that's fine - it's not like it did load before. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] Add PCI device IDs for family 17h, model 70h 2019-09-06 14:50 ` Borislav Petkov @ 2019-09-06 23:27 ` Isaac Vaughn 0 siblings, 0 replies; 18+ messages in thread From: Isaac Vaughn @ 2019-09-06 23:27 UTC (permalink / raw) To: Borislav Petkov Cc: Isaac Vaughn, Guenter Roeck, Yazen Ghannam, trivial, linux-edac, x86, linux-hwmon, linux-pci > > They seem to have already covered the changes to hwmon and amd_nb, so > > the difference is probably the changes to amd64_edac. > > Yeah, then you can send me only those changes against edac-for-next. > The driver won't load on Zen2 before the amd_nb.c changes have landed > upstream but that's fine - it's not like it did load before. > > Thx. I made the changes against the git repo you linked and sent the patch. The updated module didn't do anything when tested, but that seems to be the expected behavior with the current amd_nb. Thanks, Isaac Vaughn ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] Add PCI device IDs for family 17h, model 70h 2019-09-06 13:09 ` Boris Petkov [not found] ` <B08C8E54-43FA-4E29-8D7D-5F9C4AF20CCF@Knights.ucf.edu> @ 2019-09-06 16:11 ` Guenter Roeck 2019-09-06 16:22 ` Borislav Petkov 1 sibling, 1 reply; 18+ messages in thread From: Guenter Roeck @ 2019-09-06 16:11 UTC (permalink / raw) To: Boris Petkov Cc: Isaac Vaughn, Yazen Ghannam, trivial, linux-edac, x86, linux-hwmon, linux-pci On Fri, Sep 06, 2019 at 03:09:16PM +0200, Boris Petkov wrote: > On September 6, 2019 3:02:06 PM GMT+02:00, Guenter Roeck <linux@roeck-us.net> wrote: > >Also, how is this patch different to the patches already in linux-next > >? > > Which are those? Care to share their linux-next commit IDs with us? > 12163cfbfc0f ("hwmon: (k10temp) Add support for AMD family 17h, model 70h CPUs") af4e1c5eca95 ("x86/amd_nb: Add PCI device IDs for family 17h, model 70h") Guenter ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] Add PCI device IDs for family 17h, model 70h 2019-09-06 16:11 ` Guenter Roeck @ 2019-09-06 16:22 ` Borislav Petkov 0 siblings, 0 replies; 18+ messages in thread From: Borislav Petkov @ 2019-09-06 16:22 UTC (permalink / raw) To: Guenter Roeck Cc: Isaac Vaughn, Yazen Ghannam, trivial, linux-edac, x86, linux-hwmon, linux-pci On Fri, Sep 06, 2019 at 09:11:40AM -0700, Guenter Roeck wrote: > 12163cfbfc0f ("hwmon: (k10temp) Add support for AMD family 17h, model 70h CPUs") > af4e1c5eca95 ("x86/amd_nb: Add PCI device IDs for family 17h, model 70h") Thanks! -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2019-09-06 23:27 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-04 21:13 [PATCH] amd64_edac - Add Family 17h Model 70h PCI IDs Isaac Vaughn 2019-09-04 21:53 ` Borislav Petkov 2019-09-04 22:21 ` Ghannam, Yazen 2019-09-05 1:21 ` Isaac Vaughn 2019-09-05 7:09 ` Borislav Petkov 2019-09-05 13:17 ` Isaac Vaughn 2019-09-05 13:54 ` Borislav Petkov 2019-09-06 1:01 ` [PATCH] Add PCI device IDs for family 17h, model 70h Isaac Vaughn 2019-09-06 1:41 ` Ghannam, Yazen 2019-09-06 1:56 ` Isaac Vaughn 2019-09-06 9:12 ` Borislav Petkov [not found] ` <20190906075729.9e2faf7147da62fc26006833@knights.ucf.edu> 2019-09-06 12:14 ` Borislav Petkov 2019-09-06 13:02 ` Guenter Roeck 2019-09-06 13:09 ` Boris Petkov [not found] ` <B08C8E54-43FA-4E29-8D7D-5F9C4AF20CCF@Knights.ucf.edu> 2019-09-06 14:50 ` Borislav Petkov 2019-09-06 23:27 ` Isaac Vaughn 2019-09-06 16:11 ` Guenter Roeck 2019-09-06 16:22 ` Borislav Petkov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).