[v5,2/4] PCI: move Alibaba Vendor ID linux/pci_ids.h

Message ID 20230522035428.69441-3-xueshuai@linux.alibaba.com
State New
Headers
Series None |

Commit Message

Shuai Xue May 22, 2023, 3:54 a.m. UTC
  Move Alibaba Vendor ID (0x1ded) to linux/pci_ids.h so that it can shared by
several drivers.

Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
---
 drivers/infiniband/hw/erdma/erdma_hw.h | 2 --
 include/linux/pci_ids.h                | 2 ++
 2 files changed, 2 insertions(+), 2 deletions(-)
  

Comments

Bjorn Helgaas May 22, 2023, 4:04 p.m. UTC | #1
Please follow subject line capitalization style (learn it with "git
log --oneline include/linux/pci_ids.h"):

  PCI: Add Alibaba Vendor ID

On Mon, May 22, 2023 at 11:54:26AM +0800, Shuai Xue wrote:
> Move Alibaba Vendor ID (0x1ded) to linux/pci_ids.h so that it can shared by
> several drivers.

It would be helpful for reviewers to list the drivers here, since only
one is obvious from the patch.

Thanks for sorting the entry correctly!

> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
> ---
>  drivers/infiniband/hw/erdma/erdma_hw.h | 2 --
>  include/linux/pci_ids.h                | 2 ++
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/erdma/erdma_hw.h b/drivers/infiniband/hw/erdma/erdma_hw.h
> index 76ce2856be28..ee35ebef9ee7 100644
> --- a/drivers/infiniband/hw/erdma/erdma_hw.h
> +++ b/drivers/infiniband/hw/erdma/erdma_hw.h
> @@ -11,8 +11,6 @@
>  #include <linux/types.h>
>  
>  /* PCIe device related definition. */
> -#define PCI_VENDOR_ID_ALIBABA 0x1ded
> -
>  #define ERDMA_PCI_WIDTH 64
>  #define ERDMA_FUNC_BAR 0
>  #define ERDMA_MISX_BAR 2
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 95f33dadb2be..9e8aec472f06 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2586,6 +2586,8 @@
>  #define PCI_VENDOR_ID_TEKRAM		0x1de1
>  #define PCI_DEVICE_ID_TEKRAM_DC290	0xdc29
>  
> +#define PCI_VENDOR_ID_ALIBABA		0x1ded
> +
>  #define PCI_VENDOR_ID_TEHUTI		0x1fc9
>  #define PCI_DEVICE_ID_TEHUTI_3009	0x3009
>  #define PCI_DEVICE_ID_TEHUTI_3010	0x3010
> -- 
> 2.20.1.12.g72788fdb
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
  
Shuai Xue May 23, 2023, 3:22 a.m. UTC | #2
On 2023/5/23 00:04, Bjorn Helgaas wrote:
> Please follow subject line capitalization style (learn it with "git
> log --oneline include/linux/pci_ids.h"):
> 
>   PCI: Add Alibaba Vendor ID

Sorry, I will rewrite the subject.

> 
> On Mon, May 22, 2023 at 11:54:26AM +0800, Shuai Xue wrote:
>> Move Alibaba Vendor ID (0x1ded) to linux/pci_ids.h so that it can shared by
>> several drivers.
> 
> It would be helpful for reviewers to list the drivers here, since only
> one is obvious from the patch.

Will add it.

Then, the commit log should be:


PCI: Add Alibaba Vendor ID to linux/pci_ids.h

The Alibaba Vendor ID (0x1ded) is now only used by Alibaba elasticRDMA
adapter driver. Move the Vendor ID to linux/pci_ids.h so that it can shared
by several drivers later.

> 
> Thanks for sorting the entry correctly!

Aha, you are welcome :)

Thank you for valuable comments.

Best Regards,
Shuai

> 
>> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
>> ---
>>  drivers/infiniband/hw/erdma/erdma_hw.h | 2 --
>>  include/linux/pci_ids.h                | 2 ++
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/infiniband/hw/erdma/erdma_hw.h b/drivers/infiniband/hw/erdma/erdma_hw.h
>> index 76ce2856be28..ee35ebef9ee7 100644
>> --- a/drivers/infiniband/hw/erdma/erdma_hw.h
>> +++ b/drivers/infiniband/hw/erdma/erdma_hw.h
>> @@ -11,8 +11,6 @@
>>  #include <linux/types.h>
>>  
>>  /* PCIe device related definition. */
>> -#define PCI_VENDOR_ID_ALIBABA 0x1ded
>> -
>>  #define ERDMA_PCI_WIDTH 64
>>  #define ERDMA_FUNC_BAR 0
>>  #define ERDMA_MISX_BAR 2
>> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
>> index 95f33dadb2be..9e8aec472f06 100644
>> --- a/include/linux/pci_ids.h
>> +++ b/include/linux/pci_ids.h
>> @@ -2586,6 +2586,8 @@
>>  #define PCI_VENDOR_ID_TEKRAM		0x1de1
>>  #define PCI_DEVICE_ID_TEKRAM_DC290	0xdc29
>>  
>> +#define PCI_VENDOR_ID_ALIBABA		0x1ded
>> +
>>  #define PCI_VENDOR_ID_TEHUTI		0x1fc9
>>  #define PCI_DEVICE_ID_TEHUTI_3009	0x3009
>>  #define PCI_DEVICE_ID_TEHUTI_3010	0x3010
>> -- 
>> 2.20.1.12.g72788fdb
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
  
Bjorn Helgaas May 23, 2023, 11:54 a.m. UTC | #3
On Tue, May 23, 2023 at 11:22:08AM +0800, Shuai Xue wrote:

> The Alibaba Vendor ID (0x1ded) is now only used by Alibaba elasticRDMA
> adapter driver. Move the Vendor ID to linux/pci_ids.h so that it can shared
> by several drivers later.

Well, not exactly.  We don't want to merge changes that might be used
by unspecified drivers later.  We only want to merge things that are
needed *now*, i.e., when this complete series is merged.

In this case, I think it will be used by another driver that is part
of this series ("dwc_pcie_pmu"), so the commit log should mention both
Alibaba elasticRDMA ("erdma"?) and "dwc_pcie_pmu".

Bjorn
  
Shuai Xue May 23, 2023, 12:49 p.m. UTC | #4
On 2023/5/23 19:54, Bjorn Helgaas wrote:
> On Tue, May 23, 2023 at 11:22:08AM +0800, Shuai Xue wrote:
> 
>> The Alibaba Vendor ID (0x1ded) is now only used by Alibaba elasticRDMA
>> adapter driver. Move the Vendor ID to linux/pci_ids.h so that it can shared
>> by several drivers later.
> 
> Well, not exactly.  We don't want to merge changes that might be used
> by unspecified drivers later.  We only want to merge things that are
> needed *now*, i.e., when this complete series is merged.
>
> 
> In this case, I think it will be used by another driver that is part
> of this series ("dwc_pcie_pmu"), so the commit log should mention both
> Alibaba elasticRDMA ("erdma"?) and "dwc_pcie_pmu".
> 
> Bjorn

Yes, I have noticed the policy in head of include/linux/pci_ids.h.

	>  *	Do not add new entries to this file unless the definitions
	>  *	are shared between multiple drivers.

Actually, I mentioned both Alibaba elasticRDMA ("erdma") and PCIe PMU
"dwc_pcie_pmu" in initial draft. But I realized that dwc_pcie_pmu
is still in review, so I dropped it finally. :(

Anyway, I will add it back. Hope you are happy the bellow changes:

PCI: Add Alibaba Vendor ID to linux/pci_ids.h

The Alibaba Vendor ID (0x1ded) is now used by Alibaba elasticRDMA ("erdma") and
will be shared with the upcoming PCIe PMU ("dwc_pcie_pmu"). Move the Vendor ID
to linux/pci_ids.h so that it can shared by several drivers later.

Thank you.

Best Regards,
Shuai
  

Patch

diff --git a/drivers/infiniband/hw/erdma/erdma_hw.h b/drivers/infiniband/hw/erdma/erdma_hw.h
index 76ce2856be28..ee35ebef9ee7 100644
--- a/drivers/infiniband/hw/erdma/erdma_hw.h
+++ b/drivers/infiniband/hw/erdma/erdma_hw.h
@@ -11,8 +11,6 @@ 
 #include <linux/types.h>
 
 /* PCIe device related definition. */
-#define PCI_VENDOR_ID_ALIBABA 0x1ded
-
 #define ERDMA_PCI_WIDTH 64
 #define ERDMA_FUNC_BAR 0
 #define ERDMA_MISX_BAR 2
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 95f33dadb2be..9e8aec472f06 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2586,6 +2586,8 @@ 
 #define PCI_VENDOR_ID_TEKRAM		0x1de1
 #define PCI_DEVICE_ID_TEKRAM_DC290	0xdc29
 
+#define PCI_VENDOR_ID_ALIBABA		0x1ded
+
 #define PCI_VENDOR_ID_TEHUTI		0x1fc9
 #define PCI_DEVICE_ID_TEHUTI_3009	0x3009
 #define PCI_DEVICE_ID_TEHUTI_3010	0x3010