Message ID | 20221101021052.7532-1-slark_xiao@163.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp2676857wru; Mon, 31 Oct 2022 19:20:20 -0700 (PDT) X-Google-Smtp-Source: AMsMyM6iTG7OZgAUgditKn8ZAMeSzZaGmgIb0ZzQVy0pz0DrgQtkmOK/K6g4bnXxQaQKTL5cn5vC X-Received: by 2002:a17:90a:d244:b0:213:aa62:7a91 with SMTP id o4-20020a17090ad24400b00213aa627a91mr15906749pjw.101.1667269220569; Mon, 31 Oct 2022 19:20:20 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1667269220; cv=none; d=google.com; s=arc-20160816; b=vzd0rZzG7FteqQGUIsQVBqH9B8+gGhCfnxhf5gaWrcCZALc+FVNq74SC0h9ULYz3Oj qhnpUAGjrQDWRvyUcYwmSE3f3s5QirwAxJ7YTmqNpgmsHiUFPasjTDwnYB4hgEzll1We 3EBVOAwW+2tjJ0J3acR7FE4woQDpSl/zFbA0QIhV+fK4zIc3Q1b8wxsMQfa0l8r2aDWp GxJnCaEz72VbQm+vS5ozh3N5Tp9zZp0goA5tO/FuKWyhPebBzzisaKZIz9myP/VaXkWg KOkVc9HIugC54wnSOn5/YoRhykRnBpKb65EQNLK0fs1orHRG4Jh1iQcDYK5t+0wbUJRF 34Tg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=7JpQDTRO14UjkY6Gjl6llkqWx+hpMjtf/mA9QfbxVYo=; b=scOh1BjQg+ZfJymxPwmWKjBSdwjHr/ZQJLb3WBDeBUJCTY9UGYxgllwZVBt3TwL6BQ ZQJ4TLs63R2EKZAJGfohN2GQjw14tzpmDo/3ij7hTLwq+qoZLZJ9GxUtr390JaPkBbXG 5Dg2yh3MH0+Jy3Zkic5p9Ok2AiLZuMQOuOVRuVA0fA8PnZBxdVJuzXJgcfoWh3LlKGEM Zofxa/uaGEUXvRdyXdY8pwypLkC6OdcXWC1/4Q5mIlbpWp3FGuDacTKemzEkLYtWWlZj CFuZLSxeEFl0alMs39I2szhcepkB8VsKU+2ClKrHiHL60NTPMXQgb3RkyOW1h3wOQMf1 TACQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@163.com header.s=s110527 header.b=NEYZvPs+; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=163.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id p8-20020a170902ebc800b0016f1c879eacsi10562179plg.109.2022.10.31.19.20.08; Mon, 31 Oct 2022 19:20:20 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@163.com header.s=s110527 header.b=NEYZvPs+; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=163.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229874AbiKACML (ORCPT <rfc822;kartikey406@gmail.com> + 99 others); Mon, 31 Oct 2022 22:12:11 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54182 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229527AbiKACMI (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 31 Oct 2022 22:12:08 -0400 Received: from m12-17.163.com (m12-17.163.com [220.181.12.17]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id D12B31741A; Mon, 31 Oct 2022 19:12:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=163.com; s=s110527; h=From:Subject:Date:Message-Id:MIME-Version; bh=7JpQD TRO14UjkY6Gjl6llkqWx+hpMjtf/mA9QfbxVYo=; b=NEYZvPs+lnWZj6E/h2SOM jjrc0sT35QhVLoYtD4SzoekZsYv+QWsV/1SIHFxMHp5du2U8iE+OXO75LMhr3EE+ ON8H3bkahsaNIwo8heXSR+BTRB/oM0lzwFCVLLXutG8taR+0cFX5X5wEgmKokDDr QAnkS8hOaXom5q05pkah9k= Received: from jbd-ThinkPad-X1-Nano-Gen-1.. (unknown [223.104.68.52]) by smtp13 (Coremail) with SMTP id EcCowABHzI85gGBjaxd2mw--.40980S2; Tue, 01 Nov 2022 10:11:07 +0800 (CST) From: Slark Xiao <slark_xiao@163.com> To: mani@kernel.org, quic_hemantk@quicinc.com, bhelgaas@google.com, gregkh@linuxfoundation.org, loic.poulain@linaro.org Cc: dnlplm@gmail.com, yonglin.tan@outlook.com, mhi@lists.linux.dev, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org, Slark Xiao <slark_xiao@163.com> Subject: [PATCH v3] PCI: Add vendor ID for QUECTEL Date: Tue, 1 Nov 2022 10:10:52 +0800 Message-Id: <20221101021052.7532-1-slark_xiao@163.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-CM-TRANSID: EcCowABHzI85gGBjaxd2mw--.40980S2 X-Coremail-Antispam: 1Uf129KBjvJXoW7KrWDWr1xCw1kZw47WF1rZwb_yoW8trWrpF s09ryvvF4qqrWUtw1kK3ykXF98ZanxCF9FkFyagw4FgFsFya1Fqr9FvrWYyryagFWvqF4a qF1DurZ8G3WqyF7anT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDUYxBIdaVFxhVjvjDU0xZFpf9x0pEZXOZUUUUU= X-Originating-IP: [223.104.68.52] X-CM-SenderInfo: xvod2y5b0lt0i6rwjhhfrp/1tbiRxusZFc7ZI139AAAsS X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,SPF_HELO_NONE, SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1748258490484356792?= X-GMAIL-MSGID: =?utf-8?q?1748258490484356792?= |
Series |
[v3] PCI: Add vendor ID for QUECTEL
|
|
Commit Message
Slark Xiao
Nov. 1, 2022, 2:10 a.m. UTC
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(-)
Comments
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
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
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
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.
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.
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
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