Re:Re: Re: [PATCH v3] PCI: Add vendor ID for QUECTEL

[Slark Xiao]

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.