In 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
and https://pciids.sourceforge.net/pci.ids . Thales use Cinterion as
their IOT modem card's trademark. So you will find 0x1269 belongs to
Thales. Actually, Cinterion belongs to Gemalto, and Gemalto belongs to
Thales.
Signed-off-by: Slark Xiao <[email protected]>
---
include/linux/pci_ids.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index b362d90eb9b0..9e2b6286f53f 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -1765,6 +1765,8 @@
#define PCI_VENDOR_ID_SATSAGEM 0x1267
#define PCI_DEVICE_ID_SATSAGEM_NICCY 0x1016
+#define PCI_VENDOR_ID_CINTERION 0x1269 /* Celluar Modules*/
+
#define PCI_VENDOR_ID_ENSONIQ 0x1274
#define PCI_DEVICE_ID_ENSONIQ_CT5880 0x5880
#define PCI_DEVICE_ID_ENSONIQ_ES1370 0x5000
@@ -2585,6 +2587,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
On Fri, Oct 28, 2022 at 10:37:11AM +0800, Slark Xiao wrote:
> In 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
> and https://pciids.sourceforge.net/pci.ids . Thales use Cinterion as
> their IOT modem card's trademark. So you will find 0x1269 belongs to
> Thales. Actually, Cinterion belongs to Gemalto, and Gemalto belongs to
> Thales.
>
The patch description is confusing me.
What about below instead?
```
Add missing vendor ID for Cinterion (which is 0x1269).
```
> Signed-off-by: Slark Xiao <[email protected]>
> ---
> include/linux/pci_ids.h | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index b362d90eb9b0..9e2b6286f53f 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -1765,6 +1765,8 @@
> #define PCI_VENDOR_ID_SATSAGEM 0x1267
> #define PCI_DEVICE_ID_SATSAGEM_NICCY 0x1016
>
> +#define PCI_VENDOR_ID_CINTERION 0x1269 /* Celluar Modules*/
> +
> #define PCI_VENDOR_ID_ENSONIQ 0x1274
> #define PCI_DEVICE_ID_ENSONIQ_CT5880 0x5880
> #define PCI_DEVICE_ID_ENSONIQ_ES1370 0x5000
> @@ -2585,6 +2587,8 @@
> #define PCI_VENDOR_ID_TEKRAM 0x1de1
> #define PCI_DEVICE_ID_TEKRAM_DC290 0xdc29
>
> +#define PCI_VENDOR_ID_QUECTEL 0x1eac
> +
Why pourring in Quectel ID while this patch is about Cinterion?
Thanks.
--
An old man doll... just what I always wanted! - Clara
On Fri, Oct 28, 2022 at 10:37:11AM +0800, Slark Xiao wrote:
> In 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
> and https://pciids.sourceforge.net/pci.ids . Thales use Cinterion as
> their IOT modem card's trademark. So you will find 0x1269 belongs to
> Thales. Actually, Cinterion belongs to Gemalto, and Gemalto belongs to
> Thales.
There should not be spaces at the beginning of these lines.
Don't bother with the sourceforge URL; I don't think that's really
useful here.
> Signed-off-by: Slark Xiao <[email protected]>
> ---
> include/linux/pci_ids.h | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index b362d90eb9b0..9e2b6286f53f 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -1765,6 +1765,8 @@
> #define PCI_VENDOR_ID_SATSAGEM 0x1267
> #define PCI_DEVICE_ID_SATSAGEM_NICCY 0x1016
>
> +#define PCI_VENDOR_ID_CINTERION 0x1269 /* Celluar Modules*/
This should identify the *vendor*, not a trademark for a specific
product line. And it should correspond somehow with the PCI-SIG
registration. So I think PCI_VENDOR_ID_THALES would be more
appropriate here.
I think the best thing here would be two patches. One patch would add
PCI_VENDOR_ID_THALES to pci_ids.h and also add a use of it in the MHI
driver. The second patch would do the same for PCI_VENDOR_ID_QUECTEL.
Then each one is logically self-contained.
> #define PCI_VENDOR_ID_ENSONIQ 0x1274
> #define PCI_DEVICE_ID_ENSONIQ_CT5880 0x5880
> #define PCI_DEVICE_ID_ENSONIQ_ES1370 0x5000
> @@ -2585,6 +2587,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
>
At 2022-10-29 00:06:21, "Bjorn Helgaas" <[email protected]> wrote:
>On Fri, Oct 28, 2022 at 10:37:11AM +0800, Slark Xiao wrote:
>> In 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
>> and https://pciids.sourceforge.net/pci.ids . Thales use Cinterion as
>> their IOT modem card's trademark. So you will find 0x1269 belongs to
>> Thales. Actually, Cinterion belongs to Gemalto, and Gemalto belongs to
>> Thales.
>
>There should not be spaces at the beginning of these lines.
>
>Don't bother with the sourceforge URL; I don't think that's really
>useful here.
The space issue will be fixed in next patch.
>
>> Signed-off-by: Slark Xiao <[email protected]>
>> ---
>> include/linux/pci_ids.h | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
>> index b362d90eb9b0..9e2b6286f53f 100644
>> --- a/include/linux/pci_ids.h
>> +++ b/include/linux/pci_ids.h
>> @@ -1765,6 +1765,8 @@
>> #define PCI_VENDOR_ID_SATSAGEM 0x1267
>> #define PCI_DEVICE_ID_SATSAGEM_NICCY 0x1016
>>
>> +#define PCI_VENDOR_ID_CINTERION 0x1269 /* Celluar Modules*/
>
>This should identify the *vendor*, not a trademark for a specific
>product line. And it should correspond somehow with the PCI-SIG
>registration. So I think PCI_VENDOR_ID_THALES would be more
>appropriate here.
>
Yeah, currently it's used by IOT modules which support PCIE. But we
can't know if they will use this VID for other non-IOT modules product.
Thales would be better.
>I think the best thing here would be two patches. One patch would add
>PCI_VENDOR_ID_THALES to pci_ids.h and also add a use of it in the MHI
>driver. The second patch would do the same for PCI_VENDOR_ID_QUECTEL.
>
>Then each one is logically self-contained.
I must make sure these ids are applied, then I could commit the changes in MHI
driver side. Otherwise it will cause build error.
So I combine QUECTEL with THALES as a single patch. Shall I separate it from
each other? 1 for PCI IDs, and another for MHI change. It would be better, I think.
>
>> #define PCI_VENDOR_ID_ENSONIQ 0x1274
>> #define PCI_DEVICE_ID_ENSONIQ_CT5880 0x5880
>> #define PCI_DEVICE_ID_ENSONIQ_ES1370 0x5000
>> @@ -2585,6 +2587,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
>>
On Mon, Oct 31, 2022 at 05:08:18PM +0800, Slark Xiao wrote:
> At 2022-10-29 00:06:21, "Bjorn Helgaas" <[email protected]> wrote:
> >I think the best thing here would be two patches. One patch would add
> >PCI_VENDOR_ID_THALES to pci_ids.h and also add a use of it in the MHI
> >driver. The second patch would do the same for PCI_VENDOR_ID_QUECTEL.
> >
> >Then each one is logically self-contained.
>
> I must make sure these ids are applied, then I could commit the
> changes in MHI driver side. Otherwise it will cause build error.
>
> So I combine QUECTEL with THALES as a single patch. Shall I separate
> it from each other? 1 for PCI IDs, and another for MHI change. It
> would be better, I think.
Make two patches:
- A single patch that adds PCI_VENDOR_ID_THALES to pci_ids.h and
also adds a use of it in the MHI driver.
- A single patch that adds PCI_VENDOR_ID_QUECTEL to pci_ids.h and
also adds a use of it in the MHI driver.
That way there's no possibility of build error and no need to
coordinate patches.
Similar previous work:
https://git.kernel.org/linus/fae74feacd2d ("hisi_acc_qm: Move VF PCI device IDs to common header")
https://git.kernel.org/linus/d3826a95222c ("nfp: add support for NFP3800/NFP3803 PCIe devices")
https://git.kernel.org/linus/4fb0abfee424 ("x86/amd_nb: Add AMD Family 19h Models (10h-1Fh) and (A0h-AFh) PCI IDs")