Re: [PATCH] x86/amd_nb: add AMD family 19h model 50h PCI ids

[Guenter Roeck <linux@roeck-us.net>]

On 5/21/21 9:05 AM, Wei Huang wrote:
> 
> 
> On 5/21/21 5:10 AM, Borislav Petkov wrote:
>> On Thu, May 20, 2021 at 10:41:30AM -0700, David Bartley wrote:
>>> This is required to support Zen3 APUs in k10temp.
>>>
>>> Signed-off-by: David Bartley <andareed@gmail.com>
>>> ---
>>>   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 09083094eb57..23dda362dc0f 100644
>>> --- a/arch/x86/kernel/amd_nb.c
>>> +++ b/arch/x86/kernel/amd_nb.c
>>> @@ -25,6 +25,7 @@
>>>   #define PCI_DEVICE_ID_AMD_17H_M60H_DF_F4 0x144c
>>>   #define PCI_DEVICE_ID_AMD_17H_M70H_DF_F4 0x1444
>>>   #define PCI_DEVICE_ID_AMD_19H_DF_F4    0x1654
>>> +#define PCI_DEVICE_ID_AMD_19H_M50H_DF_F4 0x166e
>>>   /* Protect the PCI config register pairs used for SMN and DF indirect access. */
>>>   static DEFINE_MUTEX(smn_mutex);
>>> @@ -57,6 +58,7 @@ static const struct pci_device_id amd_nb_misc_ids[] = {
>>>       { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_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_19H_DF_F3) },
>>> +    { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M50H_DF_F3) },
>>>       {}
>>>   };
>>> @@ -72,6 +74,7 @@ static const struct pci_device_id amd_nb_link_ids[] = {
>>>       { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M60H_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_19H_DF_F4) },
>>> +    { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M50H_DF_F4) },
>>>       { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F4) },
>>>       {}
>>>   };
>>> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
>>> index 4c3fa5293d76..5356ccf1c275 100644
>>> --- a/include/linux/pci_ids.h
>>> +++ b/include/linux/pci_ids.h
>>> @@ -555,6 +555,7 @@
>>>   #define PCI_DEVICE_ID_AMD_17H_M60H_DF_F3 0x144b
>>>   #define PCI_DEVICE_ID_AMD_17H_M70H_DF_F3 0x1443
>>>   #define PCI_DEVICE_ID_AMD_19H_DF_F3    0x1653
>>> +#define PCI_DEVICE_ID_AMD_19H_M50H_DF_F3 0x166d
>>
>> I don't see this define used anywhere else besides amd_nb.c. If there's
>> no use for it outside of that file, I'm moving it there...?
>>
>> Or does it need to get added to that k10temp_id_table in k10temp.c too?
> 
> [+Guenter]
> 
> I think it needs to be added to k10temp.c as well. In the meanwhile, k10temp_probe() shall be extended with support for family 19h, model 0x5x.
> 

Yes, it does [1]. Fine with me though to define it locally there.
I personally find that easier since it would avoid the recurring
"I don't see this define used anywhere else besides xxx".
Someone else can clean that up later if so desired.

Thanks,
Guenter

---
[1] https://patchwork.kernel.org/project/linux-hwmon/patch/20210517064131.4369-1-andareed@gmail.com/