| Message ID | 1627909100-83338-2-git-send-email-Sanju.Mehta@amd.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | Add support for AMD USB4 and bug fixes | expand |
On Mon, Aug 02, 2021 at 07:58:17AM -0500, Sanjay R Mehta wrote: > From: Sanjay R Mehta <sanju.mehta@amd.com> > > This patch enables support for AMD USB4 host router. > > Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com> > Signed-off-by: Sanjay R Mehta <sanju.mehta@amd.com> > --- > drivers/thunderbolt/nhi.c | 4 ++++ > include/linux/pci_ids.h | 2 ++ > 2 files changed, 6 insertions(+) > > diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c > index fa44332..d7d9c4b 100644 > --- a/drivers/thunderbolt/nhi.c > +++ b/drivers/thunderbolt/nhi.c > @@ -1338,6 +1338,10 @@ static struct pci_device_id nhi_ids[] = { > { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ADL_NHI1), > .driver_data = (kernel_ulong_t)&icl_nhi_ops }, > > + /* AMD USB4 host */ > + { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_USB4_HIA0) }, > + { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_USB4_HIA1) }, > + > /* Any USB4 compliant host */ > { PCI_DEVICE_CLASS(PCI_CLASS_SERIAL_USB_USB4, ~0) }, > > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h > index 4bac183..6d50019 100644 > --- a/include/linux/pci_ids.h > +++ b/include/linux/pci_ids.h > @@ -604,6 +604,8 @@ > #define PCI_DEVICE_ID_AMD_HUDSON2_SMBUS 0x780b > #define PCI_DEVICE_ID_AMD_HUDSON2_IDE 0x780c > #define PCI_DEVICE_ID_AMD_KERNCZ_SMBUS 0x790b > +#define PCI_DEVICE_ID_AMD_USB4_HIA0 0x162e > +#define PCI_DEVICE_ID_AMD_USB4_HIA1 0x162f No need to add them here (and you actually should not since these IDs are not shared between multiple drivers, see the top level comment in this header). I suggest adding these to drivers/thunderbolt/nhi.h instead. > > #define PCI_VENDOR_ID_TRIDENT 0x1023 > #define PCI_DEVICE_ID_TRIDENT_4DWAVE_DX 0x2000 > -- > 2.7.4
On 8/2/2021 8:42 PM, Mika Westerberg wrote: > [CAUTION: External Email] > > On Mon, Aug 02, 2021 at 07:58:17AM -0500, Sanjay R Mehta wrote: >> From: Sanjay R Mehta <sanju.mehta@amd.com> >> >> This patch enables support for AMD USB4 host router. >> >> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com> >> Signed-off-by: Sanjay R Mehta <sanju.mehta@amd.com> >> --- >> drivers/thunderbolt/nhi.c | 4 ++++ >> include/linux/pci_ids.h | 2 ++ >> 2 files changed, 6 insertions(+) >> >> diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c >> index fa44332..d7d9c4b 100644 >> --- a/drivers/thunderbolt/nhi.c >> +++ b/drivers/thunderbolt/nhi.c >> @@ -1338,6 +1338,10 @@ static struct pci_device_id nhi_ids[] = { >> { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ADL_NHI1), >> .driver_data = (kernel_ulong_t)&icl_nhi_ops }, >> >> + /* AMD USB4 host */ >> + { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_USB4_HIA0) }, >> + { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_USB4_HIA1) }, >> + >> /* Any USB4 compliant host */ >> { PCI_DEVICE_CLASS(PCI_CLASS_SERIAL_USB_USB4, ~0) }, >> >> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h >> index 4bac183..6d50019 100644 >> --- a/include/linux/pci_ids.h >> +++ b/include/linux/pci_ids.h >> @@ -604,6 +604,8 @@ >> #define PCI_DEVICE_ID_AMD_HUDSON2_SMBUS 0x780b >> #define PCI_DEVICE_ID_AMD_HUDSON2_IDE 0x780c >> #define PCI_DEVICE_ID_AMD_KERNCZ_SMBUS 0x790b >> +#define PCI_DEVICE_ID_AMD_USB4_HIA0 0x162e >> +#define PCI_DEVICE_ID_AMD_USB4_HIA1 0x162f > > No need to add them here (and you actually should not since these IDs > are not shared between multiple drivers, see the top level comment in > this header). > > I suggest adding these to drivers/thunderbolt/nhi.h instead. > Thanks Mika. Agree and will add it in drivers/thunderbolt/nhi.h . >> >> #define PCI_VENDOR_ID_TRIDENT 0x1023 >> #define PCI_DEVICE_ID_TRIDENT_4DWAVE_DX 0x2000 >> -- >> 2.7.4
Hi, I have one more comment, see below. On Mon, Aug 02, 2021 at 11:39:32PM +0530, Sanjay R Mehta wrote: > > > On 8/2/2021 8:42 PM, Mika Westerberg wrote: > > [CAUTION: External Email] > > > > On Mon, Aug 02, 2021 at 07:58:17AM -0500, Sanjay R Mehta wrote: > >> From: Sanjay R Mehta <sanju.mehta@amd.com> > >> > >> This patch enables support for AMD USB4 host router. > >> > >> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com> > >> Signed-off-by: Sanjay R Mehta <sanju.mehta@amd.com> > >> --- > >> drivers/thunderbolt/nhi.c | 4 ++++ > >> include/linux/pci_ids.h | 2 ++ > >> 2 files changed, 6 insertions(+) > >> > >> diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c > >> index fa44332..d7d9c4b 100644 > >> --- a/drivers/thunderbolt/nhi.c > >> +++ b/drivers/thunderbolt/nhi.c > >> @@ -1338,6 +1338,10 @@ static struct pci_device_id nhi_ids[] = { > >> { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ADL_NHI1), > >> .driver_data = (kernel_ulong_t)&icl_nhi_ops }, > >> > >> + /* AMD USB4 host */ > >> + { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_USB4_HIA0) }, > >> + { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_USB4_HIA1) }, > >> + I wonder if AMD USB4 controller exposes the USB4 PCI class ID? If that's the case and you don't need any special quirking like Intel does then we don't need to add any PCI IDs here and allow the below line to match. This is actually what I hope we get eventually in Intel HW too. > >> /* Any USB4 compliant host */ > >> { PCI_DEVICE_CLASS(PCI_CLASS_SERIAL_USB_USB4, ~0) },
On 8/3/2021 3:15 PM, Mika Westerberg wrote: > [CAUTION: External Email] > > Hi, > > I have one more comment, see below. > > On Mon, Aug 02, 2021 at 11:39:32PM +0530, Sanjay R Mehta wrote: >> >> >> On 8/2/2021 8:42 PM, Mika Westerberg wrote: >>> [CAUTION: External Email] >>> >>> On Mon, Aug 02, 2021 at 07:58:17AM -0500, Sanjay R Mehta wrote: >>>> From: Sanjay R Mehta <sanju.mehta@amd.com> >>>> >>>> This patch enables support for AMD USB4 host router. >>>> >>>> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com> >>>> Signed-off-by: Sanjay R Mehta <sanju.mehta@amd.com> >>>> --- >>>> drivers/thunderbolt/nhi.c | 4 ++++ >>>> include/linux/pci_ids.h | 2 ++ >>>> 2 files changed, 6 insertions(+) >>>> >>>> diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c >>>> index fa44332..d7d9c4b 100644 >>>> --- a/drivers/thunderbolt/nhi.c >>>> +++ b/drivers/thunderbolt/nhi.c >>>> @@ -1338,6 +1338,10 @@ static struct pci_device_id nhi_ids[] = { >>>> { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ADL_NHI1), >>>> .driver_data = (kernel_ulong_t)&icl_nhi_ops }, >>>> >>>> + /* AMD USB4 host */ >>>> + { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_USB4_HIA0) }, >>>> + { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_USB4_HIA1) }, >>>> + > > I wonder if AMD USB4 controller exposes the USB4 PCI class ID? If that's > the case and you don't need any special quirking like Intel does then we > don't need to add any PCI IDs here and allow the below line to match. > > This is actually what I hope we get eventually in Intel HW too. yes, make sense. Will remove this from the series. Thanks, Sanjay > >>>> /* Any USB4 compliant host */ >>>> { PCI_DEVICE_CLASS(PCI_CLASS_SERIAL_USB_USB4, ~0) },
diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c index fa44332..d7d9c4b 100644 --- a/drivers/thunderbolt/nhi.c +++ b/drivers/thunderbolt/nhi.c @@ -1338,6 +1338,10 @@ static struct pci_device_id nhi_ids[] = { { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ADL_NHI1), .driver_data = (kernel_ulong_t)&icl_nhi_ops }, + /* AMD USB4 host */ + { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_USB4_HIA0) }, + { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_USB4_HIA1) }, + /* Any USB4 compliant host */ { PCI_DEVICE_CLASS(PCI_CLASS_SERIAL_USB_USB4, ~0) }, diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h index 4bac183..6d50019 100644 --- a/include/linux/pci_ids.h +++ b/include/linux/pci_ids.h @@ -604,6 +604,8 @@ #define PCI_DEVICE_ID_AMD_HUDSON2_SMBUS 0x780b #define PCI_DEVICE_ID_AMD_HUDSON2_IDE 0x780c #define PCI_DEVICE_ID_AMD_KERNCZ_SMBUS 0x790b +#define PCI_DEVICE_ID_AMD_USB4_HIA0 0x162e +#define PCI_DEVICE_ID_AMD_USB4_HIA1 0x162f #define PCI_VENDOR_ID_TRIDENT 0x1023 #define PCI_DEVICE_ID_TRIDENT_4DWAVE_DX 0x2000