* [PATCH v3] PCI: Add vendor ID for QUECTEL
@ 2022-11-01 2:10 Slark Xiao
2022-11-01 4:46 ` Greg KH
0 siblings, 1 reply; 7+ messages in thread
From: Slark Xiao @ 2022-11-01 2:10 UTC (permalink / raw)
To: mani, quic_hemantk, bhelgaas, gregkh, loic.poulain
Cc: dnlplm, yonglin.tan, mhi, linux-arm-msm, linux-kernel, linux-pci,
Slark Xiao
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@163.com>
---
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
+
#define PCI_VENDOR_ID_TEHUTI 0x1fc9
#define PCI_DEVICE_ID_TEHUTI_3009 0x3009
#define PCI_DEVICE_ID_TEHUTI_3010 0x3010
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3] PCI: Add vendor ID for QUECTEL
2022-11-01 2:10 [PATCH v3] PCI: Add vendor ID for QUECTEL Slark Xiao
@ 2022-11-01 4:46 ` Greg KH
2022-11-01 6:09 ` Slark Xiao
0 siblings, 1 reply; 7+ messages in thread
From: Greg KH @ 2022-11-01 4:46 UTC (permalink / raw)
To: Slark Xiao
Cc: mani, quic_hemantk, bhelgaas, loic.poulain, dnlplm, yonglin.tan,
mhi, linux-arm-msm, linux-kernel, linux-pci
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@163.com>
> ---
> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re:Re: [PATCH v3] PCI: Add vendor ID for QUECTEL
2022-11-01 4:46 ` Greg KH
@ 2022-11-01 6:09 ` Slark Xiao
2022-11-01 6:24 ` Greg KH
2022-11-01 14:10 ` Bjorn Helgaas
0 siblings, 2 replies; 7+ messages in thread
From: Slark Xiao @ 2022-11-01 6:09 UTC (permalink / raw)
To: Greg KH
Cc: mani, quic_hemantk, bhelgaas, loic.poulain, dnlplm, yonglin.tan,
mhi, linux-arm-msm, linux-kernel, linux-pci
At 2022-11-01 12:46:19, "Greg KH" <gregkh@linuxfoundation.org> 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@163.com>
>> ---
>> 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.
Thanks
Slark Xiao
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Re: [PATCH v3] PCI: Add vendor ID for QUECTEL
2022-11-01 6:09 ` Slark Xiao
@ 2022-11-01 6:24 ` Greg KH
2022-11-01 6:52 ` Slark Xiao
2022-11-01 14:10 ` Bjorn Helgaas
1 sibling, 1 reply; 7+ messages in thread
From: Greg KH @ 2022-11-01 6:24 UTC (permalink / raw)
To: Slark Xiao
Cc: mani, quic_hemantk, bhelgaas, loic.poulain, dnlplm, yonglin.tan,
mhi, linux-arm-msm, linux-kernel, linux-pci
On Tue, Nov 01, 2022 at 02:09:57PM +0800, Slark Xiao wrote:
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> At 2022-11-01 12:46:19, "Greg KH" <gregkh@linuxfoundation.org> 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@163.com>
> >> ---
> >> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re:Re: Re: [PATCH v3] PCI: Add vendor ID for QUECTEL
2022-11-01 6:24 ` Greg KH
@ 2022-11-01 6:52 ` Slark Xiao
2022-11-01 9:13 ` Manivannan Sadhasivam
0 siblings, 1 reply; 7+ messages in thread
From: Slark Xiao @ 2022-11-01 6:52 UTC (permalink / raw)
To: Greg KH, Manivannan Sadhasivam
Cc: quic_hemantk, bhelgaas, loic.poulain, dnlplm, yonglin.tan, mhi,
linux-arm-msm, linux-kernel, linux-pci
At 2022-11-01 14:24:58, "Greg KH" <gregkh@linuxfoundation.org> wrote:
>On Tue, Nov 01, 2022 at 02:09:57PM +0800, Slark Xiao wrote:
>>
>>
>> At 2022-11-01 12:46:19, "Greg KH" <gregkh@linuxfoundation.org> 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@163.com>
>> >> ---
>> >> 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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Re: Re: [PATCH v3] PCI: Add vendor ID for QUECTEL
2022-11-01 6:52 ` Slark Xiao
@ 2022-11-01 9:13 ` Manivannan Sadhasivam
0 siblings, 0 replies; 7+ messages in thread
From: Manivannan Sadhasivam @ 2022-11-01 9:13 UTC (permalink / raw)
To: Slark Xiao
Cc: Greg KH, quic_hemantk, bhelgaas, loic.poulain, dnlplm,
yonglin.tan, mhi, linux-arm-msm, linux-kernel, linux-pci
On Tue, Nov 01, 2022 at 02:52:45PM +0800, Slark Xiao wrote:
>
>
>
>
>
> At 2022-11-01 14:24:58, "Greg KH" <gregkh@linuxfoundation.org> wrote:
> >On Tue, Nov 01, 2022 at 02:09:57PM +0800, Slark Xiao wrote:
> >>
> >>
> >> At 2022-11-01 12:46:19, "Greg KH" <gregkh@linuxfoundation.org> 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@163.com>
> >> >> ---
> >> >> 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?
>
It is not really needed but for convenience you could add a macro in
pci_generic.c itself.
Thanks,
Mani
> Thanks.
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Re: [PATCH v3] PCI: Add vendor ID for QUECTEL
2022-11-01 6:09 ` Slark Xiao
2022-11-01 6:24 ` Greg KH
@ 2022-11-01 14:10 ` Bjorn Helgaas
1 sibling, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2022-11-01 14:10 UTC (permalink / raw)
To: Slark Xiao
Cc: Greg KH, mani, quic_hemantk, bhelgaas, loic.poulain, dnlplm,
yonglin.tan, mhi, linux-arm-msm, linux-kernel, linux-pci
On Tue, Nov 01, 2022 at 02:09:57PM +0800, Slark Xiao wrote:
> At 2022-11-01 12:46:19, "Greg KH" <gregkh@linuxfoundation.org> 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@163.com>
> >> ---
> >> 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?
>
> 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.
Sorry, this is my fault. I have merged or acked several vendor ID
additions recently, but I don't really backport changes like Greg
does, so I'm not as sensitized to the churn and merge issues.
Bjorn
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-11-01 14:10 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-01 2:10 [PATCH v3] PCI: Add vendor ID for QUECTEL Slark Xiao
2022-11-01 4:46 ` Greg KH
2022-11-01 6:09 ` Slark Xiao
2022-11-01 6:24 ` Greg KH
2022-11-01 6:52 ` Slark Xiao
2022-11-01 9:13 ` Manivannan Sadhasivam
2022-11-01 14:10 ` Bjorn Helgaas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).