| Message ID | 20240815151240.3132382-1-richard.gong@amd.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | x86/amd_nb: Add a new PCI ID for AMD family 1Ah model 60h | expand |
On Thu, Aug 15, 2024 at 10:12:40AM -0500, Richard Gong wrote: > Add a new PCI ID for Device 18h and Function 4. > > Signed-off-by: Richard Gong <richard.gong@amd.com> > --- > (Without this device ID, amd-atl driver failed to load) "amd-atl" does not appear in the source, so I don't know what it is. > --- > arch/x86/kernel/amd_nb.c | 1 + > include/linux/pci_ids.h | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c > index 61eadde08511..7566d2c079c2 100644 > --- a/arch/x86/kernel/amd_nb.c > +++ b/arch/x86/kernel/amd_nb.c > @@ -125,6 +125,7 @@ static const struct pci_device_id amd_nb_link_ids[] = { > { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M78H_DF_F4) }, > { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F4) }, > { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M00H_DF_F4) }, > + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M60H_DF_F4) }, > { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI200_DF_F4) }, > { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI300_DF_F4) }, > {} > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h > index 91182aa1d2ec..d7abfa5beaec 100644 > --- a/include/linux/pci_ids.h > +++ b/include/linux/pci_ids.h > @@ -581,6 +581,7 @@ > #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_M60H_DF_F4 0x124c From include/linux/pci_ids.h: * Do not add new entries to this file unless the definitions * are shared between multiple drivers. Maybe there's some value in having this definition in pci_ids.h as opposed to adding a definition in amd_nb.c, where there are many similar definitions? Can't tell from this commit log. Obviously this isn't adding any new *functionality*, so it would be nice if amd_nb.c could be written so it would require updates only when the programming model changes, not for every new chip. Preaching to the choir, I know. > #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.43.0 >
On Thu, Aug 15, 2024 at 10:12:40AM -0500, Richard Gong wrote: > Add a new PCI ID for Device 18h and Function 4. > > Signed-off-by: Richard Gong <richard.gong@amd.com> > --- > (Without this device ID, amd-atl driver failed to load) > --- > arch/x86/kernel/amd_nb.c | 1 + > include/linux/pci_ids.h | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c > index 61eadde08511..7566d2c079c2 100644 > --- a/arch/x86/kernel/amd_nb.c > +++ b/arch/x86/kernel/amd_nb.c > @@ -125,6 +125,7 @@ static const struct pci_device_id amd_nb_link_ids[] = { > { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M78H_DF_F4) }, > { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F4) }, > { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M00H_DF_F4) }, > + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M60H_DF_F4) }, > { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI200_DF_F4) }, > { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI300_DF_F4) }, > {} > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h > index 91182aa1d2ec..d7abfa5beaec 100644 > --- a/include/linux/pci_ids.h > +++ b/include/linux/pci_ids.h > @@ -581,6 +581,7 @@ > #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_M60H_DF_F4 0x124c This is only used in amd_nb.c, so it should be defined there. There is already a list of "F4" IDs where this can be included. Thanks, Yazen
Hi Bjorn, On 8/15/2024 11:54 AM, Bjorn Helgaas wrote: > On Thu, Aug 15, 2024 at 10:12:40AM -0500, Richard Gong wrote: >> Add a new PCI ID for Device 18h and Function 4. >> >> Signed-off-by: Richard Gong <richard.gong@amd.com> >> --- >> (Without this device ID, amd-atl driver failed to load) > > "amd-atl" does not appear in the source, so I don't know what it is. Sorry for my typo, it is amd_atl (AMD address translation library) driver. > >> --- >> arch/x86/kernel/amd_nb.c | 1 + >> include/linux/pci_ids.h | 1 + >> 2 files changed, 2 insertions(+) >> >> diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c >> index 61eadde08511..7566d2c079c2 100644 >> --- a/arch/x86/kernel/amd_nb.c >> +++ b/arch/x86/kernel/amd_nb.c >> @@ -125,6 +125,7 @@ static const struct pci_device_id amd_nb_link_ids[] = { >> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M78H_DF_F4) }, >> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F4) }, >> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M00H_DF_F4) }, >> + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M60H_DF_F4) }, >> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI200_DF_F4) }, >> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI300_DF_F4) }, >> {} >> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h >> index 91182aa1d2ec..d7abfa5beaec 100644 >> --- a/include/linux/pci_ids.h >> +++ b/include/linux/pci_ids.h >> @@ -581,6 +581,7 @@ >> #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_M60H_DF_F4 0x124c > > From include/linux/pci_ids.h: > > * Do not add new entries to this file unless the definitions > * are shared between multiple drivers. > > Maybe there's some value in having this definition in pci_ids.h as > opposed to adding a definition in amd_nb.c, where there are many > similar definitions? > > Can't tell from this commit log. > > Obviously this isn't adding any new *functionality*, so it would be > nice if amd_nb.c could be written so it would require updates only > when the programming model changes, not for every new chip. > > Preaching to the choir, I know. > >> #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.43.0 >> Regards, Richard
diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c index 61eadde08511..7566d2c079c2 100644 --- a/arch/x86/kernel/amd_nb.c +++ b/arch/x86/kernel/amd_nb.c @@ -125,6 +125,7 @@ static const struct pci_device_id amd_nb_link_ids[] = { { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M78H_DF_F4) }, { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F4) }, { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M00H_DF_F4) }, + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M60H_DF_F4) }, { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI200_DF_F4) }, { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI300_DF_F4) }, {} diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h index 91182aa1d2ec..d7abfa5beaec 100644 --- a/include/linux/pci_ids.h +++ b/include/linux/pci_ids.h @@ -581,6 +581,7 @@ #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_M60H_DF_F4 0x124c #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
Add a new PCI ID for Device 18h and Function 4. Signed-off-by: Richard Gong <richard.gong@amd.com> --- (Without this device ID, amd-atl driver failed to load) --- arch/x86/kernel/amd_nb.c | 1 + include/linux/pci_ids.h | 1 + 2 files changed, 2 insertions(+)