diff mbox

[V3,2/9] powerpc/pci_dn: cache vf_index in pci_dn

Message ID 1430723258-21299-3-git-send-email-weiyang@linux.vnet.ibm.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Wei Yang May 4, 2015, 7:07 a.m. UTC
This patch caches the index of a VF in its PF in pci_dn.

Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/pci-bridge.h |    1 +
 arch/powerpc/kernel/pci_dn.c          |    5 +++--
 2 files changed, 4 insertions(+), 2 deletions(-)

Comments

Gavin Shan May 11, 2015, 2:21 a.m. UTC | #1
On Mon, May 04, 2015 at 03:07:31PM +0800, Wei Yang wrote:
>This patch caches the index of a VF in its PF in pci_dn.
>

At least you can mention the purpose of vf_index to make the commit log
complete. The following message looks better?

The patch caches the VF index in pci_dn, which can be used to calculate
VF's bus, device and function number. Those information helps to locate
the VF's PCI device instance when doing hotplug during EEH recovery if
necessary.

>Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>---
> arch/powerpc/include/asm/pci-bridge.h |    1 +
> arch/powerpc/kernel/pci_dn.c          |    5 +++--
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
>index 1811c44..9582aa2 100644
>--- a/arch/powerpc/include/asm/pci-bridge.h
>+++ b/arch/powerpc/include/asm/pci-bridge.h
>@@ -199,6 +199,7 @@ struct pci_dn {
> #ifdef CONFIG_PCI_IOV
> 	u16     vfs_expanded;		/* number of VFs IOV BAR expanded */
> 	u16     num_vfs;		/* number of VFs enabled*/
>+	int     vf_index;		/* Index to PF for VF dev */
                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^

					/* VF index in the PF */

And I believe it can be "unsigned int", or u16. We should have
non-negative vf_index, no?

> 	int     offset;			/* PE# for the first VF PE */
> #define M64_PER_IOV 4
> 	int     m64_per_iov;
>diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
>index b3b4df9..bf0fb873 100644
>--- a/arch/powerpc/kernel/pci_dn.c
>+++ b/arch/powerpc/kernel/pci_dn.c
>@@ -138,7 +138,7 @@ struct pci_dn *pci_get_pdn(struct pci_dev *pdev)
>
> #ifdef CONFIG_PCI_IOV
> static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
>-					   struct pci_dev *pdev,
>+					   struct pci_dev *pdev, int vf_index,

					   struct pci_dev *pdev,
					   int vf_index;

> 					   int busno, int devfn)
> {
> 	struct pci_dn *pdn;
>@@ -157,6 +157,7 @@ static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
> 	pdn->parent = parent;
> 	pdn->busno = busno;
> 	pdn->devfn = devfn;
>+	pdn->vf_index = vf_index;
> #ifdef CONFIG_PPC_POWERNV
> 	pdn->pe_number = IODA_INVALID_PE;
> #endif
>@@ -196,7 +197,7 @@ struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
> 		return NULL;
>
> 	for (i = 0; i < pci_sriov_get_totalvfs(pdev); i++) {
>-		pdn = add_one_dev_pci_data(parent, NULL,
>+		pdn = add_one_dev_pci_data(parent, NULL, i,
> 					   pci_iov_virtfn_bus(pdev, i),
> 					   pci_iov_virtfn_devfn(pdev, i));
> 		if (!pdn) {

Thanks,
Gavin

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wei Yang May 11, 2015, 5:54 a.m. UTC | #2
On Mon, May 11, 2015 at 12:21:04PM +1000, Gavin Shan wrote:
>On Mon, May 04, 2015 at 03:07:31PM +0800, Wei Yang wrote:
>>This patch caches the index of a VF in its PF in pci_dn.
>>
>
>At least you can mention the purpose of vf_index to make the commit log
>complete. The following message looks better?
>
>The patch caches the VF index in pci_dn, which can be used to calculate
>VF's bus, device and function number. Those information helps to locate
>the VF's PCI device instance when doing hotplug during EEH recovery if
>necessary.
>

Thanks, looks better. I added it in the log.

>>Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>>---
>> arch/powerpc/include/asm/pci-bridge.h |    1 +
>> arch/powerpc/kernel/pci_dn.c          |    5 +++--
>> 2 files changed, 4 insertions(+), 2 deletions(-)
>>
>>diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
>>index 1811c44..9582aa2 100644
>>--- a/arch/powerpc/include/asm/pci-bridge.h
>>+++ b/arch/powerpc/include/asm/pci-bridge.h
>>@@ -199,6 +199,7 @@ struct pci_dn {
>> #ifdef CONFIG_PCI_IOV
>> 	u16     vfs_expanded;		/* number of VFs IOV BAR expanded */
>> 	u16     num_vfs;		/* number of VFs enabled*/
>>+	int     vf_index;		/* Index to PF for VF dev */
>                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
>					/* VF index in the PF */

Ok, changed in the code.

>
>And I believe it can be "unsigned int", or u16. We should have
>non-negative vf_index, no?

Take a look in the virtfn_add(), the index in drivers/pci/iov.c is int. So I
copy that.

>
>> 	int     offset;			/* PE# for the first VF PE */
>> #define M64_PER_IOV 4
>> 	int     m64_per_iov;
>>diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
>>index b3b4df9..bf0fb873 100644
>>--- a/arch/powerpc/kernel/pci_dn.c
>>+++ b/arch/powerpc/kernel/pci_dn.c
>>@@ -138,7 +138,7 @@ struct pci_dn *pci_get_pdn(struct pci_dev *pdev)
>>
>> #ifdef CONFIG_PCI_IOV
>> static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
>>-					   struct pci_dev *pdev,
>>+					   struct pci_dev *pdev, int vf_index,
>
>					   struct pci_dev *pdev,
>					   int vf_index;

Some reason for this comment?

That does not exceed 80 characters.

>
>> 					   int busno, int devfn)
>> {
>> 	struct pci_dn *pdn;
>>@@ -157,6 +157,7 @@ static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
>> 	pdn->parent = parent;
>> 	pdn->busno = busno;
>> 	pdn->devfn = devfn;
>>+	pdn->vf_index = vf_index;
>> #ifdef CONFIG_PPC_POWERNV
>> 	pdn->pe_number = IODA_INVALID_PE;
>> #endif
>>@@ -196,7 +197,7 @@ struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
>> 		return NULL;
>>
>> 	for (i = 0; i < pci_sriov_get_totalvfs(pdev); i++) {
>>-		pdn = add_one_dev_pci_data(parent, NULL,
>>+		pdn = add_one_dev_pci_data(parent, NULL, i,
>> 					   pci_iov_virtfn_bus(pdev, i),
>> 					   pci_iov_virtfn_devfn(pdev, i));
>> 		if (!pdn) {
>
>Thanks,
>Gavin
Gavin Shan May 12, 2015, 6:15 a.m. UTC | #3
On Mon, May 11, 2015 at 01:54:12PM +0800, Wei Yang wrote:
>On Mon, May 11, 2015 at 12:21:04PM +1000, Gavin Shan wrote:
>>On Mon, May 04, 2015 at 03:07:31PM +0800, Wei Yang wrote:
>>>This patch caches the index of a VF in its PF in pci_dn.
>>>
>>
>>At least you can mention the purpose of vf_index to make the commit log
>>complete. The following message looks better?
>>
>>The patch caches the VF index in pci_dn, which can be used to calculate
>>VF's bus, device and function number. Those information helps to locate
>>the VF's PCI device instance when doing hotplug during EEH recovery if
>>necessary.
>>
>
>Thanks, looks better. I added it in the log.
>
>>>Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>>>---
>>> arch/powerpc/include/asm/pci-bridge.h |    1 +
>>> arch/powerpc/kernel/pci_dn.c          |    5 +++--
>>> 2 files changed, 4 insertions(+), 2 deletions(-)
>>>
>>>diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
>>>index 1811c44..9582aa2 100644
>>>--- a/arch/powerpc/include/asm/pci-bridge.h
>>>+++ b/arch/powerpc/include/asm/pci-bridge.h
>>>@@ -199,6 +199,7 @@ struct pci_dn {
>>> #ifdef CONFIG_PCI_IOV
>>> 	u16     vfs_expanded;		/* number of VFs IOV BAR expanded */
>>> 	u16     num_vfs;		/* number of VFs enabled*/
>>>+	int     vf_index;		/* Index to PF for VF dev */
>>                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>
>>					/* VF index in the PF */
>
>Ok, changed in the code.
>
>>
>>And I believe it can be "unsigned int", or u16. We should have
>>non-negative vf_index, no?
>
>Take a look in the virtfn_add(), the index in drivers/pci/iov.c is int. So I
>copy that.
>
>>
>>> 	int     offset;			/* PE# for the first VF PE */
>>> #define M64_PER_IOV 4
>>> 	int     m64_per_iov;
>>>diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
>>>index b3b4df9..bf0fb873 100644
>>>--- a/arch/powerpc/kernel/pci_dn.c
>>>+++ b/arch/powerpc/kernel/pci_dn.c
>>>@@ -138,7 +138,7 @@ struct pci_dn *pci_get_pdn(struct pci_dev *pdev)
>>>
>>> #ifdef CONFIG_PCI_IOV
>>> static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
>>>-					   struct pci_dev *pdev,
>>>+					   struct pci_dev *pdev, int vf_index,
>>
>>					   struct pci_dev *pdev,
>>					   int vf_index;
>
>Some reason for this comment?
>
>That does not exceed 80 characters.
>

No, it doesn't exceed 80 characters as you said. You take one of the following
formats, not the one you're using:

add_one_dev_pci_data(foo1, foo2,          add_one_dev_pci_data(foo1,
                     foo3, foo4,                               foo2,
                     foo5, foo6);                                :
                                                               foo6);

>>> 					   int busno, int devfn)
>>> {
>>> 	struct pci_dn *pdn;
>>>@@ -157,6 +157,7 @@ static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
>>> 	pdn->parent = parent;
>>> 	pdn->busno = busno;
>>> 	pdn->devfn = devfn;
>>>+	pdn->vf_index = vf_index;
>>> #ifdef CONFIG_PPC_POWERNV
>>> 	pdn->pe_number = IODA_INVALID_PE;
>>> #endif
>>>@@ -196,7 +197,7 @@ struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
>>> 		return NULL;
>>>
>>> 	for (i = 0; i < pci_sriov_get_totalvfs(pdev); i++) {
>>>-		pdn = add_one_dev_pci_data(parent, NULL,
>>>+		pdn = add_one_dev_pci_data(parent, NULL, i,
>>> 					   pci_iov_virtfn_bus(pdev, i),
>>> 					   pci_iov_virtfn_devfn(pdev, i));
>>> 		if (!pdn) {

Thanks,
Gavin

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wei Yang May 12, 2015, 7:29 a.m. UTC | #4
On Tue, May 12, 2015 at 04:15:58PM +1000, Gavin Shan wrote:
>On Mon, May 11, 2015 at 01:54:12PM +0800, Wei Yang wrote:
>>On Mon, May 11, 2015 at 12:21:04PM +1000, Gavin Shan wrote:
>>>On Mon, May 04, 2015 at 03:07:31PM +0800, Wei Yang wrote:
>>>>This patch caches the index of a VF in its PF in pci_dn.
>>>>
>>>
>>>At least you can mention the purpose of vf_index to make the commit log
>>>complete. The following message looks better?
>>>
>>>The patch caches the VF index in pci_dn, which can be used to calculate
>>>VF's bus, device and function number. Those information helps to locate
>>>the VF's PCI device instance when doing hotplug during EEH recovery if
>>>necessary.
>>>
>>
>>Thanks, looks better. I added it in the log.
>>
>>>>Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>>>>---
>>>> arch/powerpc/include/asm/pci-bridge.h |    1 +
>>>> arch/powerpc/kernel/pci_dn.c          |    5 +++--
>>>> 2 files changed, 4 insertions(+), 2 deletions(-)
>>>>
>>>>diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
>>>>index 1811c44..9582aa2 100644
>>>>--- a/arch/powerpc/include/asm/pci-bridge.h
>>>>+++ b/arch/powerpc/include/asm/pci-bridge.h
>>>>@@ -199,6 +199,7 @@ struct pci_dn {
>>>> #ifdef CONFIG_PCI_IOV
>>>> 	u16     vfs_expanded;		/* number of VFs IOV BAR expanded */
>>>> 	u16     num_vfs;		/* number of VFs enabled*/
>>>>+	int     vf_index;		/* Index to PF for VF dev */
>>>                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>
>>>					/* VF index in the PF */
>>
>>Ok, changed in the code.
>>
>>>
>>>And I believe it can be "unsigned int", or u16. We should have
>>>non-negative vf_index, no?
>>
>>Take a look in the virtfn_add(), the index in drivers/pci/iov.c is int. So I
>>copy that.
>>
>>>
>>>> 	int     offset;			/* PE# for the first VF PE */
>>>> #define M64_PER_IOV 4
>>>> 	int     m64_per_iov;
>>>>diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
>>>>index b3b4df9..bf0fb873 100644
>>>>--- a/arch/powerpc/kernel/pci_dn.c
>>>>+++ b/arch/powerpc/kernel/pci_dn.c
>>>>@@ -138,7 +138,7 @@ struct pci_dn *pci_get_pdn(struct pci_dev *pdev)
>>>>
>>>> #ifdef CONFIG_PCI_IOV
>>>> static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
>>>>-					   struct pci_dev *pdev,
>>>>+					   struct pci_dev *pdev, int vf_index,
>>>
>>>					   struct pci_dev *pdev,
>>>					   int vf_index;
>>
>>Some reason for this comment?
>>
>>That does not exceed 80 characters.
>>
>
>No, it doesn't exceed 80 characters as you said. You take one of the following
>formats, not the one you're using:
>
>add_one_dev_pci_data(foo1, foo2,          add_one_dev_pci_data(foo1,
>                     foo3, foo4,                               foo2,
>                     foo5, foo6);                                :
>                                                               foo6);
>

Thanks

>>>> 					   int busno, int devfn)
>>>> {
>>>> 	struct pci_dn *pdn;
>>>>@@ -157,6 +157,7 @@ static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
>>>> 	pdn->parent = parent;
>>>> 	pdn->busno = busno;
>>>> 	pdn->devfn = devfn;
>>>>+	pdn->vf_index = vf_index;
>>>> #ifdef CONFIG_PPC_POWERNV
>>>> 	pdn->pe_number = IODA_INVALID_PE;
>>>> #endif
>>>>@@ -196,7 +197,7 @@ struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
>>>> 		return NULL;
>>>>
>>>> 	for (i = 0; i < pci_sriov_get_totalvfs(pdev); i++) {
>>>>-		pdn = add_one_dev_pci_data(parent, NULL,
>>>>+		pdn = add_one_dev_pci_data(parent, NULL, i,
>>>> 					   pci_iov_virtfn_bus(pdev, i),
>>>> 					   pci_iov_virtfn_devfn(pdev, i));
>>>> 		if (!pdn) {
>
>Thanks,
>Gavin
diff mbox

Patch

diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
index 1811c44..9582aa2 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -199,6 +199,7 @@  struct pci_dn {
 #ifdef CONFIG_PCI_IOV
 	u16     vfs_expanded;		/* number of VFs IOV BAR expanded */
 	u16     num_vfs;		/* number of VFs enabled*/
+	int     vf_index;		/* Index to PF for VF dev */
 	int     offset;			/* PE# for the first VF PE */
 #define M64_PER_IOV 4
 	int     m64_per_iov;
diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index b3b4df9..bf0fb873 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -138,7 +138,7 @@  struct pci_dn *pci_get_pdn(struct pci_dev *pdev)
 
 #ifdef CONFIG_PCI_IOV
 static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
-					   struct pci_dev *pdev,
+					   struct pci_dev *pdev, int vf_index,
 					   int busno, int devfn)
 {
 	struct pci_dn *pdn;
@@ -157,6 +157,7 @@  static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
 	pdn->parent = parent;
 	pdn->busno = busno;
 	pdn->devfn = devfn;
+	pdn->vf_index = vf_index;
 #ifdef CONFIG_PPC_POWERNV
 	pdn->pe_number = IODA_INVALID_PE;
 #endif
@@ -196,7 +197,7 @@  struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
 		return NULL;
 
 	for (i = 0; i < pci_sriov_get_totalvfs(pdev); i++) {
-		pdn = add_one_dev_pci_data(parent, NULL,
+		pdn = add_one_dev_pci_data(parent, NULL, i,
 					   pci_iov_virtfn_bus(pdev, i),
 					   pci_iov_virtfn_devfn(pdev, i));
 		if (!pdn) {