From: Manivannan Sadhasivam <mani@kernel.org>
To: Slark Xiao <slark_xiao@163.com>
Cc: Greg KH <gregkh@linuxfoundation.org>,
quic_hemantk@quicinc.com, bhelgaas@google.com,
loic.poulain@linaro.org, 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
Subject: Re: Re: Re: [PATCH v3] PCI: Add vendor ID for QUECTEL
Date: Tue, 1 Nov 2022 14:43:39 +0530 [thread overview]
Message-ID: <20221101091339.GA54667@thinkpad> (raw)
In-Reply-To: <69942131.48d4.18431f6c080.Coremail.slark_xiao@163.com>
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.
--
மணிவண்ணன் சதாசிவம்
next prev parent reply other threads:[~2022-11-01 9:13 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2022-11-01 14:10 ` Bjorn Helgaas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221101091339.GA54667@thinkpad \
--to=mani@kernel.org \
--cc=bhelgaas@google.com \
--cc=dnlplm@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=loic.poulain@linaro.org \
--cc=mhi@lists.linux.dev \
--cc=quic_hemantk@quicinc.com \
--cc=slark_xiao@163.com \
--cc=yonglin.tan@outlook.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).