[PATCH] powerpc/powernv/pci: Drop pnv_phb->initialized

[Michael Ellerman]

Oliver O'Halloran <oohall at gmail.com> writes:

> The pnv_phb->initialized flag is an odd beast. It was added back in 2012 in
> commit db1266c85261 ("powerpc/powernv: Skip check on PE if necessary") to
> allow devices to be enabled even if their PE assignments hadn't been
> completed yet. I can't think of any situation where we would (or should)
> have PCI devices being enabled before their PEs are assigned, so I can only
> assume it was a workaround for a bug or some other undesirable behaviour
> from the PCI core.
>
> Since commit dc3d8f85bb57 ("powerpc/powernv/pci: Re-work bus PE
> configuration") the PE setup occurs before the PCI core allows driver to
> attach to the device so the problem should no longer exist. Even it does
> allowing the device to be enabled before we have assigned the device to a
> PE is almost certainly broken and will cause spurious EEH events so we
> should probably just remove it.
>
> It's also worth pointing out that ->initialized flag is set in
> pnv_pci_ioda_create_dbgfs() which has the entire function body wrapped in
> flag.

"body wrapped in flag." ?

I guess you meant:

"wrapped in #ifdef CONFIG_DEBUG_FS" ?

> That has the fun side effect of bypassing any other checks in
> pnv_pci_enable_device_hook() which is probably not what anybody wants.

That would only be true for CONFIG_DEBUG_FS=n builds though.

cheers

> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> index 023a4f987bb2..6ac3c637b313 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -2410,9 +2410,6 @@ static void pnv_pci_ioda_create_dbgfs(void)
>  	list_for_each_entry_safe(hose, tmp, &hose_list, list_node) {
>  		phb = hose->private_data;
>  
> -		/* Notify initialization of PHB done */
> -		phb->initialized = 1;
> -
>  		sprintf(name, "PCI%04x", hose->global_number);
>  		phb->dbgfs = debugfs_create_dir(name, powerpc_debugfs_root);
>  
> @@ -2609,17 +2606,8 @@ static resource_size_t pnv_pci_default_alignment(void)
>   */
>  static bool pnv_pci_enable_device_hook(struct pci_dev *dev)
>  {
> -	struct pnv_phb *phb = pci_bus_to_pnvhb(dev->bus);
>  	struct pci_dn *pdn;
>  
> -	/* The function is probably called while the PEs have
> -	 * not be created yet. For example, resource reassignment
> -	 * during PCI probe period. We just skip the check if
> -	 * PEs isn't ready.
> -	 */
> -	if (!phb->initialized)
> -		return true;
> -
>  	pdn = pci_get_pdn(dev);
>  	if (!pdn || pdn->pe_number == IODA_INVALID_PE)
>  		return false;
> @@ -2629,14 +2617,9 @@ static bool pnv_pci_enable_device_hook(struct pci_dev *dev)
>  
>  static bool pnv_ocapi_enable_device_hook(struct pci_dev *dev)
>  {
> -	struct pci_controller *hose = pci_bus_to_host(dev->bus);
> -	struct pnv_phb *phb = hose->private_data;
>  	struct pci_dn *pdn;
>  	struct pnv_ioda_pe *pe;
>  
> -	if (!phb->initialized)
> -		return true;
> -
>  	pdn = pci_get_pdn(dev);
>  	if (!pdn)
>  		return false;
> diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
> index 739a0b3b72e1..36d22920f5a3 100644
> --- a/arch/powerpc/platforms/powernv/pci.h
> +++ b/arch/powerpc/platforms/powernv/pci.h
> @@ -119,7 +119,6 @@ struct pnv_phb {
>  	int			flags;
>  	void __iomem		*regs;
>  	u64			regs_phys;
> -	int			initialized;
>  	spinlock_t		lock;
>  
>  #ifdef CONFIG_DEBUG_FS
> -- 
> 2.26.2