To make code neat and for convenience purpose, use macro for
some VIDs. These macros are supposed to be added to pci_ids.h.
But until the macros are used in multiple places, it is not
recommended. So adding it locally for now.
Signed-off-by: Slark Xiao <[email protected]>
---
v2: Update description and format issue
---
drivers/bus/mhi/host/pci_generic.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
index c4259cb2d289..3a789bb2f631 100644
--- a/drivers/bus/mhi/host/pci_generic.c
+++ b/drivers/bus/mhi/host/pci_generic.c
@@ -24,6 +24,10 @@
#define HEALTH_CHECK_PERIOD (HZ * 2)
+/* PCI VID definitions */
+#define PCI_VENDOR_ID_THALES 0x1269
+#define PCI_VENDOR_ID_QUECTEL 0x1eac
+
/**
* struct mhi_pci_dev_info - MHI PCI device specific information
* @config: MHI controller configuration
@@ -557,11 +561,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),
@@ -585,16 +589,16 @@ static const struct pci_device_id mhi_pci_id_table[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0d9),
.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx65_info },
/* MV31-W (Cinterion) */
- { PCI_DEVICE(0x1269, 0x00b3),
+ { PCI_DEVICE(PCI_VENDOR_ID_THALES, 0x00b3),
.driver_data = (kernel_ulong_t) &mhi_mv31_info },
/* MV31-W (Cinterion), based on new baseline */
- { PCI_DEVICE(0x1269, 0x00b4),
+ { PCI_DEVICE(PCI_VENDOR_ID_THALES, 0x00b4),
.driver_data = (kernel_ulong_t) &mhi_mv31_info },
/* MV32-WA (Cinterion) */
- { PCI_DEVICE(0x1269, 0x00ba),
+ { PCI_DEVICE(PCI_VENDOR_ID_THALES, 0x00ba),
.driver_data = (kernel_ulong_t) &mhi_mv32_info },
/* MV32-WB (Cinterion) */
- { PCI_DEVICE(0x1269, 0x00bb),
+ { PCI_DEVICE(PCI_VENDOR_ID_THALES, 0x00bb),
.driver_data = (kernel_ulong_t) &mhi_mv32_info },
{ }
};
--
2.17.1
On Mon, Nov 07, 2022 at 04:48:26PM +0800, Slark Xiao wrote:
> To make code neat and for convenience purpose, use macro for
> some VIDs. These macros are supposed to be added to pci_ids.h.
No, they are not supposed to be added there at all.
And they are not a "macro", it is a "#define".
> But until the macros are used in multiple places, it is not
> recommended. So adding it locally for now.
Again, these are not macros
thanks,
greg k-h
At 2022-11-07 17:02:29, "Greg KH" <[email protected]> wrote:
>On Mon, Nov 07, 2022 at 04:48:26PM +0800, Slark Xiao wrote:
>> To make code neat and for convenience purpose, use macro for
>> some VIDs. These macros are supposed to be added to pci_ids.h.
>
>No, they are not supposed to be added there at all.
>
>And they are not a "macro", it is a "#define".
>
>> But until the macros are used in multiple places, it is not
>> recommended. So adding it locally for now.
>
>Again, these are not macros
>
>thanks,
>
>greg k-h
Hi Greg,
Thanks for your comment.
In my opinion, MACRO almost same as a '#define'. May I know how do
you call such definition?
And could you give your comments in previous patch, not the 'final' one?
In another pci_ids patch, you break it in v3 and break it here again in v2.
Honestly, it's positive for whole project. But it's negative for contributor
and maintainer.
Finally, it's welcome to point out the error.
On Mon, Nov 07, 2022 at 05:30:56PM +0800, Slark Xiao wrote:
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> At 2022-11-07 17:02:29, "Greg KH" <[email protected]> wrote:
> >On Mon, Nov 07, 2022 at 04:48:26PM +0800, Slark Xiao wrote:
> >> To make code neat and for convenience purpose, use macro for
> >> some VIDs. These macros are supposed to be added to pci_ids.h.
> >
> >No, they are not supposed to be added there at all.
> >
> >And they are not a "macro", it is a "#define".
> >
> >> But until the macros are used in multiple places, it is not
> >> recommended. So adding it locally for now.
> >
> >Again, these are not macros
> >
> >thanks,
> >
> >greg k-h
>
> Hi Greg,
> Thanks for your comment.
> In my opinion, MACRO almost same as a '#define'. May I know how do
> you call such definition?
As I said, this is just a define, not a macro at all.
> And could you give your comments in previous patch, not the 'final' one?
I do not understand, what previous patrch? What "final" one? What is
the "latest" patch?
> In another pci_ids patch, you break it in v3 and break it here again in v2.
I broke what?
> Honestly, it's positive for whole project. But it's negative for contributor
> and maintainer.
I am totally confused here and do not understand what you are referring
to, sorry.
greg k-h
At 2022-11-07 17:53:57, "Greg KH" <[email protected]> wrote:
>On Mon, Nov 07, 2022 at 05:30:56PM +0800, Slark Xiao wrote:
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> At 2022-11-07 17:02:29, "Greg KH" <[email protected]> wrote:
>> >On Mon, Nov 07, 2022 at 04:48:26PM +0800, Slark Xiao wrote:
>> >> To make code neat and for convenience purpose, use macro for
>> >> some VIDs. These macros are supposed to be added to pci_ids.h.
>> >
>> >No, they are not supposed to be added there at all.
>> >
>> >And they are not a "macro", it is a "#define".
>> >
>> >> But until the macros are used in multiple places, it is not
>> >> recommended. So adding it locally for now.
>> >
>> >Again, these are not macros
>> >
>> >thanks,
>> >
>> >greg k-h
>>
>> Hi Greg,
>> Thanks for your comment.
>> In my opinion, MACRO almost same as a '#define'. May I know how do
>> you call such definition?
>
>As I said, this is just a define, not a macro at all.
>
>> And could you give your comments in previous patch, not the 'final' one?
>
>I do not understand, what previous patrch? What "final" one? What is
>the "latest" patch?
previous patch:
https://lore.kernel.org/lkml/[email protected]/
https://lore.kernel.org/lkml/[email protected]/
https://lore.kernel.org/lkml/[email protected]/
'final' patch:
https://lore.kernel.org/lkml/[email protected]/
https://lore.kernel.org/lkml/[email protected]/
The 'final' patch was committed according to the advice of the feature
maintainer.
>
>> In another pci_ids patch, you break it in v3 and break it here again in v2.
>
>I broke what?
You could have voiced out such comment in V1, V2 before the 'final'.
>
>> Honestly, it's positive for whole project. But it's negative for contributor
>> and maintainer.
>
>I am totally confused here and do not understand what you are referring
>to, sorry.
>
>greg k-h
On Mon, Nov 07, 2022 at 06:26:16PM +0800, Slark Xiao wrote:
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
Why all the blank lines?
> At 2022-11-07 17:53:57, "Greg KH" <[email protected]> wrote:
> >On Mon, Nov 07, 2022 at 05:30:56PM +0800, Slark Xiao wrote:
> >As I said, this is just a define, not a macro at all.
> >
> >> And could you give your comments in previous patch, not the 'final' one?
> >
> >I do not understand, what previous patrch? What "final" one? What is
> >the "latest" patch?
> previous patch:
> https://lore.kernel.org/lkml/[email protected]/
> https://lore.kernel.org/lkml/[email protected]/
> https://lore.kernel.org/lkml/[email protected]/
>
> 'final' patch:
> https://lore.kernel.org/lkml/[email protected]/
> https://lore.kernel.org/lkml/[email protected]/
That's 2 different versions, with a total of 3.
>
> The 'final' patch was committed according to the advice of the feature
> maintainer.
> >
> >> In another pci_ids patch, you break it in v3 and break it here again in v2.
> >
> >I broke what?
> You could have voiced out such comment in V1, V2 before the 'final'.
We all review patches when we can. There is no rule that people can not
comment on newer patches, or older ones.
In fact, it would be wonderful if you could take some time and review
patches from others. It would help your understanding of the code and
how the kernel development process works.
thanks,
greg k-h
At 2022-11-07 20:07:52, "Greg KH" <[email protected]> wrote:
>On Mon, Nov 07, 2022 at 06:26:16PM +0800, Slark Xiao wrote:
>Why all the blank lines?
163 mail automatically add it , and I forget remove it.
>
>> At 2022-11-07 17:53:57, "Greg KH" <[email protected]> wrote:
>> >On Mon, Nov 07, 2022 at 05:30:56PM +0800, Slark Xiao wrote:
>> >As I said, this is just a define, not a macro at all.
>> >
>> >> And could you give your comments in previous patch, not the 'final' one?
>> >
>> >I do not understand, what previous patrch? What "final" one? What is
>> >the "latest" patch?
>> previous patch:
>> https://lore.kernel.org/lkml/[email protected]/
>> https://lore.kernel.org/lkml/[email protected]/
>> https://lore.kernel.org/lkml/[email protected]/
>>
>> 'final' patch:
>> https://lore.kernel.org/lkml/[email protected]/
>> https://lore.kernel.org/lkml/[email protected]/
>
>That's 2 different versions, with a total of 3.
>
>>
>> The 'final' patch was committed according to the advice of the feature
>> maintainer.
>> >
>> >> In another pci_ids patch, you break it in v3 and break it here again in v2.
>> >
>> >I broke what?
>> You could have voiced out such comment in V1, V2 before the 'final'.
>
>We all review patches when we can. There is no rule that people can not
>comment on newer patches, or older ones.
>
>In fact, it would be wonderful if you could take some time and review
>patches from others. It would help your understanding of the code and
>how the kernel development process works.
>
>thanks,
>
>greg k-h
Agree with this. But you know, 5 attempt just for a little update were rejected
It's so frustrating. Anyway, I re-submit another v3 patch for this change.
Please give your comment if you are free.
Thanks