diff mbox series

[2/2] PCI: vmd: Make use of PCI_DEVICE_DATA() helper function

Message ID 20210722112954.477-3-caihuoqing@baidu.com (mailing list archive)
State Superseded
Delegated to: Bjorn Helgaas
Headers show
Series PCI: Make use of PCI_DEVICE_XXX() helper function | expand

Commit Message

Cai,Huoqing July 22, 2021, 11:29 a.m. UTC
We could make use of PCI_DEVICE_DATA() helper function

Signed-off-by: Cai Huoqing <caihuoqing@baidu.com>
---
 drivers/pci/controller/vmd.c | 38 ++++++++++++++++++------------------
 include/linux/pci_ids.h      |  2 ++
 2 files changed, 21 insertions(+), 19 deletions(-)

Comments

Jon Derrick July 22, 2021, 4:45 p.m. UTC | #1
Hello,

On 7/22/2021 5:29 AM, Cai Huoqing wrote:
> We could make use of PCI_DEVICE_DATA() helper function
> 
> Signed-off-by: Cai Huoqing <caihuoqing@baidu.com>
> ---
>  drivers/pci/controller/vmd.c | 38 ++++++++++++++++++------------------
>  include/linux/pci_ids.h      |  2 ++
>  2 files changed, 21 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index e3fcdfec58b3..565681ed00a1 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -859,25 +859,25 @@ static int vmd_resume(struct device *dev)
>  static SIMPLE_DEV_PM_OPS(vmd_dev_pm_ops, vmd_suspend, vmd_resume);
>  
>  static const struct pci_device_id vmd_ids[] = {
> -       {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_VMD_201D),
> -               .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP,},
> -       {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_VMD_28C0),
> -               .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW |
> -                               VMD_FEAT_HAS_BUS_RESTRICTIONS |
> -                               VMD_FEAT_CAN_BYPASS_MSI_REMAP,},
> -       {PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x467f),
> -               .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
> -                               VMD_FEAT_HAS_BUS_RESTRICTIONS |
> -                               VMD_FEAT_OFFSET_FIRST_VECTOR,},
> -       {PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x4c3d),
> -               .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
> -                               VMD_FEAT_HAS_BUS_RESTRICTIONS |
> -                               VMD_FEAT_OFFSET_FIRST_VECTOR,},
> -       {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_VMD_9A0B),
> -               .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
> -                               VMD_FEAT_HAS_BUS_RESTRICTIONS |
> -                               VMD_FEAT_OFFSET_FIRST_VECTOR,},
> -       {0,}
> +       { PCI_DEVICE_DATA(INTEL, VMD_201D,
> +                         VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP) },
> +       { PCI_DEVICE_DATA(INTEL, VMD_28C0,
> +                         VMD_FEAT_HAS_MEMBAR_SHADOW |
> +                         VMD_FEAT_HAS_BUS_RESTRICTIONS |
> +                         VMD_FEAT_CAN_BYPASS_MSI_REMAP) },
> +       { PCI_DEVICE_DATA(INTEL, VMD_467F,
> +                         VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
> +                         VMD_FEAT_HAS_BUS_RESTRICTIONS |
> +                         VMD_FEAT_OFFSET_FIRST_VECTOR) },
> +       { PCI_DEVICE_DATA(INTEL, VMD_4C3D,
> +                         VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
> +                         VMD_FEAT_HAS_BUS_RESTRICTIONS |
> +                         VMD_FEAT_OFFSET_FIRST_VECTOR) },
> +       { PCI_DEVICE_DATA(INTEL, VMD_9A0B,
> +                         VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
> +                         VMD_FEAT_HAS_BUS_RESTRICTIONS |
> +                         VMD_FEAT_OFFSET_FIRST_VECTOR) },
> +       { },
>  };
>  MODULE_DEVICE_TABLE(pci, vmd_ids);
>  
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 4bac1831de80..d25552b5ae3e 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2954,6 +2954,8 @@
>  #define PCI_DEVICE_ID_INTEL_SBRIDGE_BR         0x3cf5  /* 13.6 */
>  #define PCI_DEVICE_ID_INTEL_SBRIDGE_SAD1       0x3cf6  /* 12.7 */
>  #define PCI_DEVICE_ID_INTEL_IOAT_SNB   0x402f
> +#define PCI_DEVICE_ID_INTEL_VMD_467F   0x467f
> +#define PCI_DEVICE_ID_INTEL_VMD_4C3D   0x4c3d
>  #define PCI_DEVICE_ID_INTEL_5100_16    0x65f0
>  #define PCI_DEVICE_ID_INTEL_5100_19    0x65f3
>  #define PCI_DEVICE_ID_INTEL_5100_21    0x65f5
> 

This is fine with me

Reviewed-by: Jon Derrick <jonathan.derrick@intel.com>
Cai,Huoqing July 23, 2021, 1:34 a.m. UTC | #2
Hello,
	PATCH[2/2] has some extra indentation, please don't apply it , 
I'll send PATCH V2.
THX

-----Original Message-----
From: Derrick, Jonathan <jonathan.derrick@intel.com> 
Sent: 2021年7月23日 0:46
To: Cai,Huoqing <caihuoqing@baidu.com>
Cc: linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] PCI: vmd: Make use of PCI_DEVICE_DATA() helper function

Hello,

On 7/22/2021 5:29 AM, Cai Huoqing wrote:
> We could make use of PCI_DEVICE_DATA() helper function
> 
> Signed-off-by: Cai Huoqing <caihuoqing@baidu.com>
> ---
>  drivers/pci/controller/vmd.c | 38 ++++++++++++++++++------------------
>  include/linux/pci_ids.h      |  2 ++
>  2 files changed, 21 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/pci/controller/vmd.c 
> b/drivers/pci/controller/vmd.c index e3fcdfec58b3..565681ed00a1 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -859,25 +859,25 @@ static int vmd_resume(struct device *dev)  
> static SIMPLE_DEV_PM_OPS(vmd_dev_pm_ops, vmd_suspend, vmd_resume);
>  
>  static const struct pci_device_id vmd_ids[] = {
> -       {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_VMD_201D),
> -               .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP,},
> -       {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_VMD_28C0),
> -               .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW |
> -                               VMD_FEAT_HAS_BUS_RESTRICTIONS |
> -                               VMD_FEAT_CAN_BYPASS_MSI_REMAP,},
> -       {PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x467f),
> -               .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
> -                               VMD_FEAT_HAS_BUS_RESTRICTIONS |
> -                               VMD_FEAT_OFFSET_FIRST_VECTOR,},
> -       {PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x4c3d),
> -               .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
> -                               VMD_FEAT_HAS_BUS_RESTRICTIONS |
> -                               VMD_FEAT_OFFSET_FIRST_VECTOR,},
> -       {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_VMD_9A0B),
> -               .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
> -                               VMD_FEAT_HAS_BUS_RESTRICTIONS |
> -                               VMD_FEAT_OFFSET_FIRST_VECTOR,},
> -       {0,}
> +       { PCI_DEVICE_DATA(INTEL, VMD_201D,
> +                         VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP) },
> +       { PCI_DEVICE_DATA(INTEL, VMD_28C0,
> +                         VMD_FEAT_HAS_MEMBAR_SHADOW |
> +                         VMD_FEAT_HAS_BUS_RESTRICTIONS |
> +                         VMD_FEAT_CAN_BYPASS_MSI_REMAP) },
> +       { PCI_DEVICE_DATA(INTEL, VMD_467F,
> +                         VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
> +                         VMD_FEAT_HAS_BUS_RESTRICTIONS |
> +                         VMD_FEAT_OFFSET_FIRST_VECTOR) },
> +       { PCI_DEVICE_DATA(INTEL, VMD_4C3D,
> +                         VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
> +                         VMD_FEAT_HAS_BUS_RESTRICTIONS |
> +                         VMD_FEAT_OFFSET_FIRST_VECTOR) },
> +       { PCI_DEVICE_DATA(INTEL, VMD_9A0B,
> +                         VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
> +                         VMD_FEAT_HAS_BUS_RESTRICTIONS |
> +                         VMD_FEAT_OFFSET_FIRST_VECTOR) },
> +       { },
>  };
>  MODULE_DEVICE_TABLE(pci, vmd_ids);
>  
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h index 
> 4bac1831de80..d25552b5ae3e 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2954,6 +2954,8 @@
>  #define PCI_DEVICE_ID_INTEL_SBRIDGE_BR         0x3cf5  /* 13.6 */
>  #define PCI_DEVICE_ID_INTEL_SBRIDGE_SAD1       0x3cf6  /* 12.7 */
>  #define PCI_DEVICE_ID_INTEL_IOAT_SNB   0x402f
> +#define PCI_DEVICE_ID_INTEL_VMD_467F   0x467f
> +#define PCI_DEVICE_ID_INTEL_VMD_4C3D   0x4c3d
>  #define PCI_DEVICE_ID_INTEL_5100_16    0x65f0
>  #define PCI_DEVICE_ID_INTEL_5100_19    0x65f3
>  #define PCI_DEVICE_ID_INTEL_5100_21    0x65f5
> 

This is fine with me

Reviewed-by: Jon Derrick <jonathan.derrick@intel.com>
Krzysztof Wilczyński July 23, 2021, 2 a.m. UTC | #3
Hi,

Thank you for sending the patch over!

> 	PATCH[2/2] has some extra indentation, please don't apply it , 

You might know this already, but just in case make sure to run the
"checkpatch.pl" script over the patch before submission.

> I'll send PATCH V2.

You mean v3?  Make sure to include a changelog, if possible.

A small nitpick: the PCI_DEVICE_DATA() is technically a macro rather
than a function, so you could update both the subject line and the
commit message accordingly, if you want.

Also, since you are about to send another version, add period at the end
of the sentence in the commit message.

Aside of the above, it's a nice refactor, thank you!

	Krzysztof
Cai,Huoqing July 23, 2021, 3:28 a.m. UTC | #4
Hi,

 	Thank for your suggestion, useful to me
I will send PATCH v3 again with changelog :

v1->v2: *fix  extra indent which occurs git-apply error

v2->v3:  *update the subject line from "funtion" to "macro"
	 *add changelog to commit message

commit date: 7-23-2021 11:00

thx

-----Original Message-----
From: Krzysztof Wilczyński <kw@linux.com> 
Sent: 2021年7月23日 10:00
To: Cai,Huoqing <caihuoqing@baidu.com>
Cc: Derrick, Jonathan <jonathan.derrick@intel.com>; linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] PCI: vmd: Make use of PCI_DEVICE_DATA() helper function

Hi,

Thank you for sending the patch over!

> 	PATCH[2/2] has some extra indentation, please don't apply it ,

You might know this already, but just in case make sure to run the "checkpatch.pl" script over the patch before submission.

> I'll send PATCH V2.

You mean v3?  Make sure to include a changelog, if possible.

A small nitpick: the PCI_DEVICE_DATA() is technically a macro rather than a function, so you could update both the subject line and the commit message accordingly, if you want.

Also, since you are about to send another version, add period at the end of the sentence in the commit message.

Aside of the above, it's a nice refactor, thank you!

	Krzysztof
diff mbox series

Patch

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index e3fcdfec58b3..565681ed00a1 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -859,25 +859,25 @@  static int vmd_resume(struct device *dev)
 static SIMPLE_DEV_PM_OPS(vmd_dev_pm_ops, vmd_suspend, vmd_resume);
 
 static const struct pci_device_id vmd_ids[] = {
-       {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_VMD_201D),
-               .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP,},
-       {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_VMD_28C0),
-               .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW |
-                               VMD_FEAT_HAS_BUS_RESTRICTIONS |
-                               VMD_FEAT_CAN_BYPASS_MSI_REMAP,},
-       {PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x467f),
-               .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
-                               VMD_FEAT_HAS_BUS_RESTRICTIONS |
-                               VMD_FEAT_OFFSET_FIRST_VECTOR,},
-       {PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x4c3d),
-               .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
-                               VMD_FEAT_HAS_BUS_RESTRICTIONS |
-                               VMD_FEAT_OFFSET_FIRST_VECTOR,},
-       {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_VMD_9A0B),
-               .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
-                               VMD_FEAT_HAS_BUS_RESTRICTIONS |
-                               VMD_FEAT_OFFSET_FIRST_VECTOR,},
-       {0,}
+       { PCI_DEVICE_DATA(INTEL, VMD_201D,
+                         VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP) },
+       { PCI_DEVICE_DATA(INTEL, VMD_28C0,
+                         VMD_FEAT_HAS_MEMBAR_SHADOW |
+                         VMD_FEAT_HAS_BUS_RESTRICTIONS |
+                         VMD_FEAT_CAN_BYPASS_MSI_REMAP) },
+       { PCI_DEVICE_DATA(INTEL, VMD_467F,
+                         VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
+                         VMD_FEAT_HAS_BUS_RESTRICTIONS |
+                         VMD_FEAT_OFFSET_FIRST_VECTOR) },
+       { PCI_DEVICE_DATA(INTEL, VMD_4C3D,
+                         VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
+                         VMD_FEAT_HAS_BUS_RESTRICTIONS |
+                         VMD_FEAT_OFFSET_FIRST_VECTOR) },
+       { PCI_DEVICE_DATA(INTEL, VMD_9A0B,
+                         VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
+                         VMD_FEAT_HAS_BUS_RESTRICTIONS |
+                         VMD_FEAT_OFFSET_FIRST_VECTOR) },
+       { },
 };
 MODULE_DEVICE_TABLE(pci, vmd_ids);
 
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 4bac1831de80..d25552b5ae3e 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2954,6 +2954,8 @@ 
 #define PCI_DEVICE_ID_INTEL_SBRIDGE_BR         0x3cf5  /* 13.6 */
 #define PCI_DEVICE_ID_INTEL_SBRIDGE_SAD1       0x3cf6  /* 12.7 */
 #define PCI_DEVICE_ID_INTEL_IOAT_SNB   0x402f
+#define PCI_DEVICE_ID_INTEL_VMD_467F   0x467f
+#define PCI_DEVICE_ID_INTEL_VMD_4C3D   0x4c3d
 #define PCI_DEVICE_ID_INTEL_5100_16    0x65f0
 #define PCI_DEVICE_ID_INTEL_5100_19    0x65f3
 #define PCI_DEVICE_ID_INTEL_5100_21    0x65f5