Subject: Re: [PATCH V3 2/9] powerpc/pci_dn: cache vf_index in pci_dn

From: Gavin Shan <[email-masked]>
Date: Mon, 11 May 2015 12:21:04 +1000

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 <[email-masked]>
>---
> 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