On Tue, Jun 10, 2014 at 09:56:29AM +0800, Wei Yang wrote:
>On PowerNV platform, it will support dynamic PE allocation and deallocation.
>
>This patch adds a function to release those resources related to a PE.
>
>Signed-off-by: Wei Yang <weiyang@xxxxxxxxxxxxxxxxxx>
>---
> arch/powerpc/platforms/powernv/pci-ioda.c | 77 +++++++++++++++++++++++++++++
> 1 file changed, 77 insertions(+)
>
>diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
>index 8ca3926..87cb3089 100644
>--- a/arch/powerpc/platforms/powernv/pci-ioda.c
>+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>@@ -330,6 +330,83 @@ static struct pnv_ioda_pe *pnv_ioda_get_pe(struct pci_dev *dev)
> }
> #endif /* CONFIG_PCI_MSI */
>
>+static int pnv_ioda_deconfigure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe)
>+{
Richard, it seems that the deconfiguration is incomplete. Something seems
missed: DMA, IO and MMIO, MSI. If I understand correctly, pnv_ioda_deconfigure_pe()
won't tear down DMA, IO and MMIO, MSI properly. For MSI/MSIx, it wouldn't
be a problem as the VF driver should disable them before calling this function.
>+ struct pci_dev *parent;
>+ uint8_t bcomp, dcomp, fcomp;
>+ int64_t rc;
>+ long rid_end, rid;
Blank line needed here to separate variable declaration and logic. And I think
we won't run into case "if (pe->pbus)" for now. So it's worthy to have some
comments to explain it for a bit :-)
>+ if (pe->pbus) {
>+ int count;
>+
>+ dcomp = OPAL_IGNORE_RID_DEVICE_NUMBER;
>+ fcomp = OPAL_IGNORE_RID_FUNCTION_NUMBER;
>+ parent = pe->pbus->self;
>+ if (pe->flags & PNV_IODA_PE_BUS_ALL)
>+ count = pe->pbus->busn_res.end - pe->pbus->busn_res.start + 1;
>+ else
>+ count = 1;
>+
>+ switch(count) {
>+ case 1: bcomp = OpalPciBusAll; break;
>+ case 2: bcomp = OpalPciBus7Bits; break;
>+ case 4: bcomp = OpalPciBus6Bits; break;
>+ case 8: bcomp = OpalPciBus5Bits; break;
>+ case 16: bcomp = OpalPciBus4Bits; break;
>+ case 32: bcomp = OpalPciBus3Bits; break;
>+ default:
>+ pr_err("%s: Number of subordinate busses %d"
>+ " unsupported\n",
>+ pci_name(pe->pbus->self), count);
I guess it's not safe to do "pci_name(pe->pbus->self)" root root bus.
>+ /* Do an exact match only */
>+ bcomp = OpalPciBusAll;
>+ }
>+ rid_end = pe->rid + (count << 8);
>+ }else {
} else {
>+ parent = pe->pdev->bus->self;
>+ bcomp = OpalPciBusAll;
>+ dcomp = OPAL_COMPARE_RID_DEVICE_NUMBER;
>+ fcomp = OPAL_COMPARE_RID_FUNCTION_NUMBER;
>+ rid_end = pe->rid + 1;
>+ }
>+
>+ /* Disable MVT on IODA1 */
>+ if (phb->type == PNV_PHB_IODA1) {
>+ rc = opal_pci_set_mve_enable(phb->opal_id,
>+ pe->mve_number, OPAL_DISABLE_MVE);
>+ if (rc) {
>+ pe_err(pe, "OPAL error %ld enabling MVE %d\n",
>+ rc, pe->mve_number);
>+ pe->mve_number = -1;
>+ }
>+ }
>+ /* Clear the reverse map */
>+ for (rid = pe->rid; rid < rid_end; rid++)
>+ phb->ioda.pe_rmap[rid] = 0;
>+
>+ /* Release from all parents PELT-V */
>+ while (parent) {
>+ struct pci_dn *pdn = pci_get_pdn(parent);
>+ if (pdn && pdn->pe_number != IODA_INVALID_PE) {
>+ rc = opal_pci_set_peltv(phb->opal_id, pdn->pe_number,
>+ pe->pe_number, OPAL_REMOVE_PE_FROM_DOMAIN);
>+ /* XXX What to do in case of error ? */
>+ }
>+ parent = parent->bus->self;
>+ }
It seems that you missed removing the PE from its own PELTV, which was
introduced by commit 631ad69 ("powerpc/powernv: Add PE to its own PELTV").
>+
>+ /* Dissociate PE in PELT */
>+ rc = opal_pci_set_pe(phb->opal_id, pe->pe_number, pe->rid,
>+ bcomp, dcomp, fcomp, OPAL_UNMAP_PE);
>+ if (rc)
>+ pe_err(pe, "OPAL error %ld trying to setup PELT table\n", rc);
>+
>+ pe->pbus = NULL;
>+ pe->pdev = NULL;
>+
>+ return 0;
>+}
>+
> static int pnv_ioda_configure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe)
> {
> struct pci_dev *parent;
Thanks,
Gavin
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
