arch/x86/kernel/amd_nb.c | 3 +++ include/linux/pci_ids.h | 1 + 2 files changed, 4 insertions(+)
Add the new PCI Device IDs to the root IDs and misc ids list to support
new generation of AMD 1Ah family 60h Models of processors.
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
(As the amd_nb functions are used by PMC and PMF drivers, without these IDs
being present AMD PMF/PMC probe shall fail.)
arch/x86/kernel/amd_nb.c | 3 +++
include/linux/pci_ids.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c
index 059e5c16af05..61eadde08511 100644
--- a/arch/x86/kernel/amd_nb.c
+++ b/arch/x86/kernel/amd_nb.c
@@ -26,6 +26,7 @@
#define PCI_DEVICE_ID_AMD_19H_M70H_ROOT 0x14e8
#define PCI_DEVICE_ID_AMD_1AH_M00H_ROOT 0x153a
#define PCI_DEVICE_ID_AMD_1AH_M20H_ROOT 0x1507
+#define PCI_DEVICE_ID_AMD_1AH_M60H_ROOT 0x1122
#define PCI_DEVICE_ID_AMD_MI200_ROOT 0x14bb
#define PCI_DEVICE_ID_AMD_MI300_ROOT 0x14f8
@@ -63,6 +64,7 @@ static const struct pci_device_id amd_root_ids[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M70H_ROOT) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M00H_ROOT) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M20H_ROOT) },
+ { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M60H_ROOT) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI200_ROOT) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI300_ROOT) },
{}
@@ -95,6 +97,7 @@ static const struct pci_device_id amd_nb_misc_ids[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M78H_DF_F3) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M00H_DF_F3) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M20H_DF_F3) },
+ { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M60H_DF_F3) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M70H_DF_F3) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI200_DF_F3) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI300_DF_F3) },
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 76a8f2d6bd64..bbe8f3dfa813 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -580,6 +580,7 @@
#define PCI_DEVICE_ID_AMD_19H_M78H_DF_F3 0x12fb
#define PCI_DEVICE_ID_AMD_1AH_M00H_DF_F3 0x12c3
#define PCI_DEVICE_ID_AMD_1AH_M20H_DF_F3 0x16fb
+#define PCI_DEVICE_ID_AMD_1AH_M60H_DF_F3 0x124b
#define PCI_DEVICE_ID_AMD_1AH_M70H_DF_F3 0x12bb
#define PCI_DEVICE_ID_AMD_MI200_DF_F3 0x14d3
#define PCI_DEVICE_ID_AMD_MI300_DF_F3 0x152b
--
2.25.1On Thu, Jul 18, 2024 at 07:32:58PM +0530, Shyam Sundar S K wrote:
> Add the new PCI Device IDs to the root IDs and misc ids list to support
> new generation of AMD 1Ah family 60h Models of processors.
>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
> (As the amd_nb functions are used by PMC and PMF drivers, without these IDs
> being present AMD PMF/PMC probe shall fail.)
Is there any plan for making this generic so a kernel update is not
needed? Obviously the *functionality* is not changed by this patch,
so having to add a device ID for every new processor just makes work
for distros and users.
> arch/x86/kernel/amd_nb.c | 3 +++
> include/linux/pci_ids.h | 1 +
> 2 files changed, 4 insertions(+)
>
> diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c
> index 059e5c16af05..61eadde08511 100644
> --- a/arch/x86/kernel/amd_nb.c
> +++ b/arch/x86/kernel/amd_nb.c
> @@ -26,6 +26,7 @@
> #define PCI_DEVICE_ID_AMD_19H_M70H_ROOT 0x14e8
> #define PCI_DEVICE_ID_AMD_1AH_M00H_ROOT 0x153a
> #define PCI_DEVICE_ID_AMD_1AH_M20H_ROOT 0x1507
> +#define PCI_DEVICE_ID_AMD_1AH_M60H_ROOT 0x1122
> #define PCI_DEVICE_ID_AMD_MI200_ROOT 0x14bb
> #define PCI_DEVICE_ID_AMD_MI300_ROOT 0x14f8
>
> @@ -63,6 +64,7 @@ static const struct pci_device_id amd_root_ids[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M70H_ROOT) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M00H_ROOT) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M20H_ROOT) },
> + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M60H_ROOT) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI200_ROOT) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI300_ROOT) },
> {}
> @@ -95,6 +97,7 @@ static const struct pci_device_id amd_nb_misc_ids[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M78H_DF_F3) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M00H_DF_F3) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M20H_DF_F3) },
> + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M60H_DF_F3) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M70H_DF_F3) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI200_DF_F3) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI300_DF_F3) },
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 76a8f2d6bd64..bbe8f3dfa813 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -580,6 +580,7 @@
> #define PCI_DEVICE_ID_AMD_19H_M78H_DF_F3 0x12fb
> #define PCI_DEVICE_ID_AMD_1AH_M00H_DF_F3 0x12c3
> #define PCI_DEVICE_ID_AMD_1AH_M20H_DF_F3 0x16fb
> +#define PCI_DEVICE_ID_AMD_1AH_M60H_DF_F3 0x124b
Why not add this in amd_nb.c, as you did for
PCI_DEVICE_ID_AMD_1AH_M60H_ROOT? There's already a
PCI_DEVICE_ID_AMD_CNB17H_F4 definition there. No need to update
pci_ids.h unless PCI_DEVICE_ID_AMD_1AH_M60H_DF_F3 is used in more than
one place.
Based on previous history, I suppose PCI_DEVICE_ID_AMD_1AH_M60H_DF_F3
will someday be used by k10temp.c? Ideally a pci_ids.h addition would
be in the same patch that adds uses in both amd_nb.c and k10temp.c so
it's clear that the new definition is used in two places.
> #define PCI_DEVICE_ID_AMD_1AH_M70H_DF_F3 0x12bb
> #define PCI_DEVICE_ID_AMD_MI200_DF_F3 0x14d3
> #define PCI_DEVICE_ID_AMD_MI300_DF_F3 0x152b
> --
> 2.25.1
>
On 7/18/2024 22:43, Bjorn Helgaas wrote:
> On Thu, Jul 18, 2024 at 07:32:58PM +0530, Shyam Sundar S K wrote:
>> Add the new PCI Device IDs to the root IDs and misc ids list to support
>> new generation of AMD 1Ah family 60h Models of processors.
>>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>> ---
>> (As the amd_nb functions are used by PMC and PMF drivers, without these IDs
>> being present AMD PMF/PMC probe shall fail.)
>
> Is there any plan for making this generic so a kernel update is not
> needed? Obviously the *functionality* is not changed by this patch,
> so having to add a device ID for every new processor just makes work
> for distros and users.
Regarding AMD processors, there are numerous PCI IDs defined in the
PPRs/BKDG. I'm not sure if there's a generic way to address this
without a kernel update.
>
>> arch/x86/kernel/amd_nb.c | 3 +++
>> include/linux/pci_ids.h | 1 +
>> 2 files changed, 4 insertions(+)
>>
>> diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c
>> index 059e5c16af05..61eadde08511 100644
>> --- a/arch/x86/kernel/amd_nb.c
>> +++ b/arch/x86/kernel/amd_nb.c
>> @@ -26,6 +26,7 @@
>> #define PCI_DEVICE_ID_AMD_19H_M70H_ROOT 0x14e8
>> #define PCI_DEVICE_ID_AMD_1AH_M00H_ROOT 0x153a
>> #define PCI_DEVICE_ID_AMD_1AH_M20H_ROOT 0x1507
>> +#define PCI_DEVICE_ID_AMD_1AH_M60H_ROOT 0x1122
>> #define PCI_DEVICE_ID_AMD_MI200_ROOT 0x14bb
>> #define PCI_DEVICE_ID_AMD_MI300_ROOT 0x14f8
>>
>> @@ -63,6 +64,7 @@ static const struct pci_device_id amd_root_ids[] = {
>> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M70H_ROOT) },
>> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M00H_ROOT) },
>> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M20H_ROOT) },
>> + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M60H_ROOT) },
>> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI200_ROOT) },
>> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI300_ROOT) },
>> {}
>> @@ -95,6 +97,7 @@ static const struct pci_device_id amd_nb_misc_ids[] = {
>> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M78H_DF_F3) },
>> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M00H_DF_F3) },
>> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M20H_DF_F3) },
>> + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M60H_DF_F3) },
>> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M70H_DF_F3) },
>> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI200_DF_F3) },
>> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI300_DF_F3) },
>> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
>> index 76a8f2d6bd64..bbe8f3dfa813 100644
>> --- a/include/linux/pci_ids.h
>> +++ b/include/linux/pci_ids.h
>> @@ -580,6 +580,7 @@
>> #define PCI_DEVICE_ID_AMD_19H_M78H_DF_F3 0x12fb
>> #define PCI_DEVICE_ID_AMD_1AH_M00H_DF_F3 0x12c3
>> #define PCI_DEVICE_ID_AMD_1AH_M20H_DF_F3 0x16fb
>> +#define PCI_DEVICE_ID_AMD_1AH_M60H_DF_F3 0x124b
>
> Why not add this in amd_nb.c, as you did for
> PCI_DEVICE_ID_AMD_1AH_M60H_ROOT? There's already a
> PCI_DEVICE_ID_AMD_CNB17H_F4 definition there. No need to update
> pci_ids.h unless PCI_DEVICE_ID_AMD_1AH_M60H_DF_F3 is used in more than
> one place.
>
> Based on previous history, I suppose PCI_DEVICE_ID_AMD_1AH_M60H_DF_F3
> will someday be used by k10temp.c? Ideally a pci_ids.h addition would
> be in the same patch that adds uses in both amd_nb.c and k10temp.c so
> it's clear that the new definition is used in two places.
>
Okay, I understand your point. I will add
PCI_DEVICE_ID_AMD_1AH_M60H_DF_F3 to k10temp.c in v2.
Thanks,
Shyam
>> #define PCI_DEVICE_ID_AMD_1AH_M70H_DF_F3 0x12bb
>> #define PCI_DEVICE_ID_AMD_MI200_DF_F3 0x14d3
>> #define PCI_DEVICE_ID_AMD_MI300_DF_F3 0x152b
>> --
>> 2.25.1
>>
On Thu, Jul 18, 2024 at 11:37:31PM +0530, Shyam Sundar S K wrote: > > > On 7/18/2024 22:43, Bjorn Helgaas wrote: > > On Thu, Jul 18, 2024 at 07:32:58PM +0530, Shyam Sundar S K wrote: > >> Add the new PCI Device IDs to the root IDs and misc ids list to support > >> new generation of AMD 1Ah family 60h Models of processors. > >> > >> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> > >> --- > >> (As the amd_nb functions are used by PMC and PMF drivers, without these IDs > >> being present AMD PMF/PMC probe shall fail.) > > > > Is there any plan for making this generic so a kernel update is not > > needed? Obviously the *functionality* is not changed by this patch, > > so having to add a device ID for every new processor just makes work > > for distros and users. > > Regarding AMD processors, there are numerous PCI IDs defined in the > PPRs/BKDG. I'm not sure if there's a generic way to address this > without a kernel update. > One of our colleagues is working on a possible solution. It'll likely use device types and class codes so we don't need to add individual PCI IDs for each new product. Thanks, Yazen
On Thu, Jul 18, 2024 at 07:32:58PM +0530, Shyam Sundar S K wrote:
> Add the new PCI Device IDs to the root IDs and misc ids list to support
> new generation of AMD 1Ah family 60h Models of processors.
Please be consistent with formatting.
"Device" -> "device"
"misc ids" -> "misc IDs"
"Models" -> "models"
Also, you have "0x1A" in the $SUBJECT, but you have "1Ah" in the commit
message. I suggest staying with "1Ah" as that is the format used in AMD
documentation.
And "v1" is not necessary in the "[PATCH]" prefix.
Furthermore, if you CC the "x86" alias, then you don't need to CC the
individual x86 maintainers.
>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
> (As the amd_nb functions are used by PMC and PMF drivers, without these IDs
> being present AMD PMF/PMC probe shall fail.)
This comment can go in the commit message. Otherwise, it'll be lost from
the git history.
The comment is helpful in that it gives a reason *why* these new IDs are
needed.
>
> arch/x86/kernel/amd_nb.c | 3 +++
> include/linux/pci_ids.h | 1 +
> 2 files changed, 4 insertions(+)
>
> diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c
> index 059e5c16af05..61eadde08511 100644
> --- a/arch/x86/kernel/amd_nb.c
> +++ b/arch/x86/kernel/amd_nb.c
> @@ -26,6 +26,7 @@
> #define PCI_DEVICE_ID_AMD_19H_M70H_ROOT 0x14e8
> #define PCI_DEVICE_ID_AMD_1AH_M00H_ROOT 0x153a
> #define PCI_DEVICE_ID_AMD_1AH_M20H_ROOT 0x1507
> +#define PCI_DEVICE_ID_AMD_1AH_M60H_ROOT 0x1122
> #define PCI_DEVICE_ID_AMD_MI200_ROOT 0x14bb
> #define PCI_DEVICE_ID_AMD_MI300_ROOT 0x14f8
>
> @@ -63,6 +64,7 @@ static const struct pci_device_id amd_root_ids[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M70H_ROOT) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M00H_ROOT) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M20H_ROOT) },
> + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M60H_ROOT) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI200_ROOT) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI300_ROOT) },
> {}
> @@ -95,6 +97,7 @@ static const struct pci_device_id amd_nb_misc_ids[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M78H_DF_F3) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M00H_DF_F3) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M20H_DF_F3) },
> + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M60H_DF_F3) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M70H_DF_F3) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI200_DF_F3) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI300_DF_F3) },
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 76a8f2d6bd64..bbe8f3dfa813 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -580,6 +580,7 @@
> #define PCI_DEVICE_ID_AMD_19H_M78H_DF_F3 0x12fb
> #define PCI_DEVICE_ID_AMD_1AH_M00H_DF_F3 0x12c3
> #define PCI_DEVICE_ID_AMD_1AH_M20H_DF_F3 0x16fb
> +#define PCI_DEVICE_ID_AMD_1AH_M60H_DF_F3 0x124b
> #define PCI_DEVICE_ID_AMD_1AH_M70H_DF_F3 0x12bb
> #define PCI_DEVICE_ID_AMD_MI200_DF_F3 0x14d3
> #define PCI_DEVICE_ID_AMD_MI300_DF_F3 0x152b
> --
I can confirm that the IDs are correct.
Besides the formatting issues, this looks good to me.
Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
Thanks,
Yazen
On 7/18/2024 21:13, Yazen Ghannam wrote:
> On Thu, Jul 18, 2024 at 07:32:58PM +0530, Shyam Sundar S K wrote:
>> Add the new PCI Device IDs to the root IDs and misc ids list to support
>> new generation of AMD 1Ah family 60h Models of processors.
>
> Please be consistent with formatting.
>
> "Device" -> "device"
>
> "misc ids" -> "misc IDs"
>
> "Models" -> "models"
>
> Also, you have "0x1A" in the $SUBJECT, but you have "1Ah" in the commit
> message. I suggest staying with "1Ah" as that is the format used in AMD
> documentation.
>
> And "v1" is not necessary in the "[PATCH]" prefix.
>
> Furthermore, if you CC the "x86" alias, then you don't need to CC the
> individual x86 maintainers.
I used get_maintainer.pl to send it. I can remove individual names and
send it only to the x86 maintainers.
>
>>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>> ---
>> (As the amd_nb functions are used by PMC and PMF drivers, without these IDs
>> being present AMD PMF/PMC probe shall fail.)
>
> This comment can go in the commit message. Otherwise, it'll be lost from
> the git history.
>
> The comment is helpful in that it gives a reason *why* these new IDs are
> needed.
>
My previous commit 0e640f0a47d8 ("x86/amd_nb: Add new PCI IDs for AMD
family 0x1a") included this note in the commit message, but Boris had
to trim it. Therefore, I excluded it this time.
Should I include or exclude this note?
I can do a re-spin based on your further remarks.
Thanks,
Shyam
>>
>> arch/x86/kernel/amd_nb.c | 3 +++
>> include/linux/pci_ids.h | 1 +
>> 2 files changed, 4 insertions(+)
>>
>> diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c
>> index 059e5c16af05..61eadde08511 100644
>> --- a/arch/x86/kernel/amd_nb.c
>> +++ b/arch/x86/kernel/amd_nb.c
>> @@ -26,6 +26,7 @@
>> #define PCI_DEVICE_ID_AMD_19H_M70H_ROOT 0x14e8
>> #define PCI_DEVICE_ID_AMD_1AH_M00H_ROOT 0x153a
>> #define PCI_DEVICE_ID_AMD_1AH_M20H_ROOT 0x1507
>> +#define PCI_DEVICE_ID_AMD_1AH_M60H_ROOT 0x1122
>> #define PCI_DEVICE_ID_AMD_MI200_ROOT 0x14bb
>> #define PCI_DEVICE_ID_AMD_MI300_ROOT 0x14f8
>>
>> @@ -63,6 +64,7 @@ static const struct pci_device_id amd_root_ids[] = {
>> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M70H_ROOT) },
>> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M00H_ROOT) },
>> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M20H_ROOT) },
>> + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M60H_ROOT) },
>> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI200_ROOT) },
>> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI300_ROOT) },
>> {}
>> @@ -95,6 +97,7 @@ static const struct pci_device_id amd_nb_misc_ids[] = {
>> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M78H_DF_F3) },
>> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M00H_DF_F3) },
>> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M20H_DF_F3) },
>> + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M60H_DF_F3) },
>> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M70H_DF_F3) },
>> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI200_DF_F3) },
>> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI300_DF_F3) },
>> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
>> index 76a8f2d6bd64..bbe8f3dfa813 100644
>> --- a/include/linux/pci_ids.h
>> +++ b/include/linux/pci_ids.h
>> @@ -580,6 +580,7 @@
>> #define PCI_DEVICE_ID_AMD_19H_M78H_DF_F3 0x12fb
>> #define PCI_DEVICE_ID_AMD_1AH_M00H_DF_F3 0x12c3
>> #define PCI_DEVICE_ID_AMD_1AH_M20H_DF_F3 0x16fb
>> +#define PCI_DEVICE_ID_AMD_1AH_M60H_DF_F3 0x124b
>> #define PCI_DEVICE_ID_AMD_1AH_M70H_DF_F3 0x12bb
>> #define PCI_DEVICE_ID_AMD_MI200_DF_F3 0x14d3
>> #define PCI_DEVICE_ID_AMD_MI300_DF_F3 0x152b
>> --
>
> I can confirm that the IDs are correct.
>
> Besides the formatting issues, this looks good to me.
>
> Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>>
> Thanks,
> Yazen
On Thu, Jul 18, 2024 at 10:19:47PM +0530, Shyam Sundar S K wrote:
>
>
> On 7/18/2024 21:13, Yazen Ghannam wrote:
> > On Thu, Jul 18, 2024 at 07:32:58PM +0530, Shyam Sundar S K wrote:
> >> Add the new PCI Device IDs to the root IDs and misc ids list to support
> >> new generation of AMD 1Ah family 60h Models of processors.
> >
> > Please be consistent with formatting.
> >
> > "Device" -> "device"
> >
> > "misc ids" -> "misc IDs"
> >
> > "Models" -> "models"
> >
> > Also, you have "0x1A" in the $SUBJECT, but you have "1Ah" in the commit
> > message. I suggest staying with "1Ah" as that is the format used in AMD
> > documentation.
> >
> > And "v1" is not necessary in the "[PATCH]" prefix.
> >
> > Furthermore, if you CC the "x86" alias, then you don't need to CC the
> > individual x86 maintainers.
>
> I used get_maintainer.pl to send it. I can remove individual names and
> send it only to the x86 maintainers.
>
Understood. I think this only applies to those who are listed as
maintainers: "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
They are already included by "x86@kernel.org", so copying them
individually is redundant. At least, that is the feedback I have
received previously.
For reference, please see "x86 architecture" here:
https://www.kernel.org/doc/html/latest/process/maintainer-tip.html
> >
> >>
> >> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> >> ---
> >> (As the amd_nb functions are used by PMC and PMF drivers, without these IDs
> >> being present AMD PMF/PMC probe shall fail.)
> >
> > This comment can go in the commit message. Otherwise, it'll be lost from
> > the git history.
> >
> > The comment is helpful in that it gives a reason *why* these new IDs are
> > needed.
> >
>
> My previous commit 0e640f0a47d8 ("x86/amd_nb: Add new PCI IDs for AMD
> family 0x1a") included this note in the commit message, but Boris had
> to trim it. Therefore, I excluded it this time.
>
> Should I include or exclude this note?
>
I see. In that case, you can exclude the note unless there is more
feedback from others.
Thanks,
Yazen
© 2016 - 2025 Red Hat, Inc.