At 2022-11-01 14:24:58, "Greg KH" <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: >On Tue, Nov 01, 2022 at 02:09:57PM +0800, Slark Xiao wrote: >> >> >> At 2022-11-01 12:46:19, "Greg KH" <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: >> >On Tue, Nov 01, 2022 at 10:10:52AM +0800, Slark Xiao wrote: >> >> n MHI driver, there are some companies' product still do not have their >> >> own PCI vendor macro. So we add it here to make the code neat. Ref ID >> >> could be found in link https://pcisig.com/membership/member-companies. >> >> >> >> Signed-off-by: Slark Xiao <slark_xiao@xxxxxxx> >> >> --- >> >> v3: Separate different vendors into different patch. >> >> >> >> v2: Update vendor ID to the right location sorted by numeric value. >> >> --- >> >> drivers/bus/mhi/host/pci_generic.c | 6 +++--- >> >> include/linux/pci_ids.h | 2 ++ >> >> 2 files changed, 5 insertions(+), 3 deletions(-) >> >> >> >> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c >> >> index caa4ce28cf9e..81ae9c49ce2a 100644 >> >> --- a/drivers/bus/mhi/host/pci_generic.c >> >> +++ b/drivers/bus/mhi/host/pci_generic.c >> >> @@ -555,11 +555,11 @@ static const struct pci_device_id mhi_pci_id_table[] = { >> >> .driver_data = (kernel_ulong_t) &mhi_telit_fn990_info }, >> >> { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0308), >> >> .driver_data = (kernel_ulong_t) &mhi_qcom_sdx65_info }, >> >> - { PCI_DEVICE(0x1eac, 0x1001), /* EM120R-GL (sdx24) */ >> >> + { PCI_DEVICE(PCI_VENDOR_ID_QUECTEL, 0x1001), /* EM120R-GL (sdx24) */ >> >> .driver_data = (kernel_ulong_t) &mhi_quectel_em1xx_info }, >> >> - { PCI_DEVICE(0x1eac, 0x1002), /* EM160R-GL (sdx24) */ >> >> + { PCI_DEVICE(PCI_VENDOR_ID_QUECTEL, 0x1002), /* EM160R-GL (sdx24) */ >> >> .driver_data = (kernel_ulong_t) &mhi_quectel_em1xx_info }, >> >> - { PCI_DEVICE(0x1eac, 0x2001), /* EM120R-GL for FCCL (sdx24) */ >> >> + { PCI_DEVICE(PCI_VENDOR_ID_QUECTEL, 0x2001), /* EM120R-GL for FCCL (sdx24) */ >> >> .driver_data = (kernel_ulong_t) &mhi_quectel_em1xx_info }, >> >> /* T99W175 (sdx55), Both for eSIM and Non-eSIM */ >> >> { PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0ab), >> >> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h >> >> index b362d90eb9b0..3c91461bcfe4 100644 >> >> --- a/include/linux/pci_ids.h >> >> +++ b/include/linux/pci_ids.h >> >> @@ -2585,6 +2585,8 @@ >> >> #define PCI_VENDOR_ID_TEKRAM 0x1de1 >> >> #define PCI_DEVICE_ID_TEKRAM_DC290 0xdc29 >> >> >> >> +#define PCI_VENDOR_ID_QUECTEL 0x1eac >> > >> >Why did you ignore the comment at the top of this file saying that new >> >entries are not needed to be added, especially for just one user? >> > >> >thanks, >> > >> >greg k-h >> Hi Greg, >> Actually I didn't see this notice before committing this patch. I even discussed >> it with the maintainer for several times and nobody show me this rule. >> I have a concern, some IOT module vendors, like QUECTEL, CINTERION(THALES), >> SIERRA,ROLLING and so on, they only produce IOT modules without other >> hardware with PCIe interface, and they applied for their own VID. But they >> can't get a their own VENDOR MARCO? This seems unreasonable. >> This change should be harmless and make the code neat. >> This is my opinion. > >It causes a _LOT_ of churn and merge issues when everyone is adding new >entries to a single file. Which is why, 15+ years ago, we made the >decision that if a vendor or device id is only needed in one file, then >it should not be added to the pci_ids.h file. > >No need to change that now, please just put the vendor id in the single >driver that it is needed in. > >thanks, > >greg k-h Hi Greg, Thanks for your explanation. Hi Mani, Is there a need to update these vendor ids as macro in pci_generic.c? Thanks.
