[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20221101141038.GA1255580@bhelgaas>
Date: Tue, 1 Nov 2022 09:10:38 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: Slark Xiao <slark_xiao@....com>
Cc: Greg KH <gregkh@...uxfoundation.org>, mani@...nel.org,
quic_hemantk@...cinc.com, bhelgaas@...gle.com,
loic.poulain@...aro.org, dnlplm@...il.com, yonglin.tan@...look.com,
mhi@...ts.linux.dev, linux-arm-msm@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org
Subject: Re: Re: [PATCH v3] PCI: Add vendor ID for QUECTEL
On Tue, Nov 01, 2022 at 02:09:57PM +0800, Slark Xiao wrote:
> At 2022-11-01 12:46:19, "Greg KH" <gregkh@...uxfoundation.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@....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
Powered by blists - more mailing lists