Re: [RFC PATCH kernel] powerpc/pci: Get rid of unused @parent pointer in pci_controller
On 06/11/17 14:33, Benjamin Herrenschmidt wrote: > On Mon, 2017-11-06 at 14:24 +1100, Alexey Kardashevskiy wrote: >> The @parent pointer is supposed to point to a device which represents >> a PCI controller, however it is never set to anything and remains NULL; >> it is also quite common to pass NULL to pci_create_root_bus(). >> >> Signed-off-by: Alexey Kardashevskiy >> --- > > It's used for anything using of_pci_phb_probe() such as Cell. ah, ok, thanks. > > >> >> This @parent is NULL for every PHB in garrison machines, for example. >> Where would this pointer be really used? >> >> I wonder how does setting it make any difference in sysfs for CXL vphb? >> >> In general, I'd think that @parent makes sense and every PHB >> has a parent device struct like any other device and the platform >> code binds a correct device driver to a PHB and we can avoid >> having PNV_PHB_IODA2&co switches but for some reason PHBs are >> hardcoded in the platform code. >> >> --- >> arch/powerpc/include/asm/pci-bridge.h | 1 - >> arch/powerpc/kernel/pci-common.c | 2 +- >> drivers/misc/cxl/vphb.c | 3 --- >> 3 files changed, 1 insertion(+), 5 deletions(-) >> >> diff --git a/arch/powerpc/include/asm/pci-bridge.h >> b/arch/powerpc/include/asm/pci-bridge.h >> index 752718a2949d..ad88ddeaa6b7 100644 >> --- a/arch/powerpc/include/asm/pci-bridge.h >> +++ b/arch/powerpc/include/asm/pci-bridge.h >> @@ -60,7 +60,6 @@ struct pci_controller { >> #endif >> struct device_node *dn; >> struct list_head list_node; >> -struct device *parent; >> >> int first_busno; >> int last_busno; >> diff --git a/arch/powerpc/kernel/pci-common.c >> b/arch/powerpc/kernel/pci-common.c >> index 02831a396419..597576777c34 100644 >> --- a/arch/powerpc/kernel/pci-common.c >> +++ b/arch/powerpc/kernel/pci-common.c >> @@ -1681,7 +1681,7 @@ void pcibios_scan_phb(struct pci_controller *hose) >> pci_add_resource(&resources, &hose->busn); >> >> /* Create an empty bus for the toplevel */ >> -bus = pci_create_root_bus(hose->parent, hose->first_busno, >> +bus = pci_create_root_bus(NULL, hose->first_busno, >>hose->ops, hose, &resources); >> if (bus == NULL) { >> pr_err("Failed to create bus for PCI domain %04x\n", >> diff --git a/drivers/misc/cxl/vphb.c b/drivers/misc/cxl/vphb.c >> index 512a4897dbf6..ae3fe1563812 100644 >> --- a/drivers/misc/cxl/vphb.c >> +++ b/drivers/misc/cxl/vphb.c >> @@ -232,9 +232,6 @@ int cxl_pci_vphb_add(struct cxl_afu *afu) >> if (!phb) >> return -ENODEV; >> >> -/* Setup parent in sysfs */ >> -phb->parent = parent; >> - >> /* Setup the PHB using arch provided callback */ >> phb->ops = &cxl_pcie_pci_ops; >> phb->cfg_addr = NULL; -- Alexey
Re: [RFC PATCH kernel] powerpc/pci: Get rid of unused @parent pointer in pci_controller
On Mon, 2017-11-06 at 14:24 +1100, Alexey Kardashevskiy wrote: > The @parent pointer is supposed to point to a device which represents > a PCI controller, however it is never set to anything and remains NULL; > it is also quite common to pass NULL to pci_create_root_bus(). > > Signed-off-by: Alexey Kardashevskiy > --- It's used for anything using of_pci_phb_probe() such as Cell. > > This @parent is NULL for every PHB in garrison machines, for example. > Where would this pointer be really used? > > I wonder how does setting it make any difference in sysfs for CXL vphb? > > In general, I'd think that @parent makes sense and every PHB > has a parent device struct like any other device and the platform > code binds a correct device driver to a PHB and we can avoid > having PNV_PHB_IODA2&co switches but for some reason PHBs are > hardcoded in the platform code. > > --- > arch/powerpc/include/asm/pci-bridge.h | 1 - > arch/powerpc/kernel/pci-common.c | 2 +- > drivers/misc/cxl/vphb.c | 3 --- > 3 files changed, 1 insertion(+), 5 deletions(-) > > diff --git a/arch/powerpc/include/asm/pci-bridge.h > b/arch/powerpc/include/asm/pci-bridge.h > index 752718a2949d..ad88ddeaa6b7 100644 > --- a/arch/powerpc/include/asm/pci-bridge.h > +++ b/arch/powerpc/include/asm/pci-bridge.h > @@ -60,7 +60,6 @@ struct pci_controller { > #endif > struct device_node *dn; > struct list_head list_node; > - struct device *parent; > > int first_busno; > int last_busno; > diff --git a/arch/powerpc/kernel/pci-common.c > b/arch/powerpc/kernel/pci-common.c > index 02831a396419..597576777c34 100644 > --- a/arch/powerpc/kernel/pci-common.c > +++ b/arch/powerpc/kernel/pci-common.c > @@ -1681,7 +1681,7 @@ void pcibios_scan_phb(struct pci_controller *hose) > pci_add_resource(&resources, &hose->busn); > > /* Create an empty bus for the toplevel */ > - bus = pci_create_root_bus(hose->parent, hose->first_busno, > + bus = pci_create_root_bus(NULL, hose->first_busno, > hose->ops, hose, &resources); > if (bus == NULL) { > pr_err("Failed to create bus for PCI domain %04x\n", > diff --git a/drivers/misc/cxl/vphb.c b/drivers/misc/cxl/vphb.c > index 512a4897dbf6..ae3fe1563812 100644 > --- a/drivers/misc/cxl/vphb.c > +++ b/drivers/misc/cxl/vphb.c > @@ -232,9 +232,6 @@ int cxl_pci_vphb_add(struct cxl_afu *afu) > if (!phb) > return -ENODEV; > > - /* Setup parent in sysfs */ > - phb->parent = parent; > - > /* Setup the PHB using arch provided callback */ > phb->ops = &cxl_pcie_pci_ops; > phb->cfg_addr = NULL;
[RFC PATCH kernel] powerpc/pci: Get rid of unused @parent pointer in pci_controller
The @parent pointer is supposed to point to a device which represents a PCI controller, however it is never set to anything and remains NULL; it is also quite common to pass NULL to pci_create_root_bus(). Signed-off-by: Alexey Kardashevskiy --- This @parent is NULL for every PHB in garrison machines, for example. Where would this pointer be really used? I wonder how does setting it make any difference in sysfs for CXL vphb? In general, I'd think that @parent makes sense and every PHB has a parent device struct like any other device and the platform code binds a correct device driver to a PHB and we can avoid having PNV_PHB_IODA2&co switches but for some reason PHBs are hardcoded in the platform code. --- arch/powerpc/include/asm/pci-bridge.h | 1 - arch/powerpc/kernel/pci-common.c | 2 +- drivers/misc/cxl/vphb.c | 3 --- 3 files changed, 1 insertion(+), 5 deletions(-) diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h index 752718a2949d..ad88ddeaa6b7 100644 --- a/arch/powerpc/include/asm/pci-bridge.h +++ b/arch/powerpc/include/asm/pci-bridge.h @@ -60,7 +60,6 @@ struct pci_controller { #endif struct device_node *dn; struct list_head list_node; - struct device *parent; int first_busno; int last_busno; diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c index 02831a396419..597576777c34 100644 --- a/arch/powerpc/kernel/pci-common.c +++ b/arch/powerpc/kernel/pci-common.c @@ -1681,7 +1681,7 @@ void pcibios_scan_phb(struct pci_controller *hose) pci_add_resource(&resources, &hose->busn); /* Create an empty bus for the toplevel */ - bus = pci_create_root_bus(hose->parent, hose->first_busno, + bus = pci_create_root_bus(NULL, hose->first_busno, hose->ops, hose, &resources); if (bus == NULL) { pr_err("Failed to create bus for PCI domain %04x\n", diff --git a/drivers/misc/cxl/vphb.c b/drivers/misc/cxl/vphb.c index 512a4897dbf6..ae3fe1563812 100644 --- a/drivers/misc/cxl/vphb.c +++ b/drivers/misc/cxl/vphb.c @@ -232,9 +232,6 @@ int cxl_pci_vphb_add(struct cxl_afu *afu) if (!phb) return -ENODEV; - /* Setup parent in sysfs */ - phb->parent = parent; - /* Setup the PHB using arch provided callback */ phb->ops = &cxl_pcie_pci_ops; phb->cfg_addr = NULL; -- 2.11.0