[PATCH v1] x86/amd_nb: Add new PCI IDs for AMD family 0x1a model 60h

[PATCH v1] x86/amd_nb: Add new PCI IDs for AMD family 0x1a model 60h

[Shyam Sundar S K]

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.1

Re: [PATCH v1] x86/amd_nb: Add new PCI IDs for AMD family 0x1a model 60h

[Yazen Ghannam]

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

Re: [PATCH v1] x86/amd_nb: Add new PCI IDs for AMD family 0x1a model 60h

[Shyam Sundar S K]

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

Re: [PATCH v1] x86/amd_nb: Add new PCI IDs for AMD family 0x1a model 60h

[Bjorn Helgaas]

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.

>  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
> 

Re: [PATCH v1] x86/amd_nb: Add new PCI IDs for AMD family 0x1a model 60h

[Shyam Sundar S K]

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
>>

Re: [PATCH v1] x86/amd_nb: Add new PCI IDs for AMD family 0x1a model 60h

[Yazen Ghannam]

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

Re: [PATCH v1] x86/amd_nb: Add new PCI IDs for AMD family 0x1a model 60h

[Yazen Ghannam]

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