[PATCH] powerpc/kernel: Change retrieval of pci_dn

[Michael Ellerman]

Hi Bryant,

Thanks for the patch, a few comments/questions.

How have you tested this?

"Bryant G. Ly" <bryantly at linux.vnet.ibm.com> writes:
> For a PCI device it's pci_dn can be retrieved from
> pdev->dev.archdata.firmware_data, PCI_DN(devnode), or parent's list.
> Thus, we should just use the generic function pci_get_pdn_by_devfn
> to get the pci_dn.

"generic" means shared between architectures, which is not true for
pci_get_pdn_by_devfn(). "existing" would be more appropriate.

> diff --git a/arch/powerpc/kernel/rtas_pci.c b/arch/powerpc/kernel/rtas_pci.c
> index 73f1934..c21e6c5 100644
> --- a/arch/powerpc/kernel/rtas_pci.c
> +++ b/arch/powerpc/kernel/rtas_pci.c
> @@ -91,25 +91,13 @@ static int rtas_pci_read_config(struct pci_bus *bus,
>  				unsigned int devfn,
>  				int where, int size, u32 *val)
>  {
> -	struct device_node *busdn, *dn;
>  	struct pci_dn *pdn;
> -	bool found = false;
>  	int ret;
>  
>  	/* Search only direct children of the bus */

You kept the comment, but is it still true, and what does it apply to
now?

Also you removed the comment below in write, I'd expect it to either
stay in both or be removed in both?

>  	*val = 0xFFFFFFFF;
> -	busdn = pci_bus_to_OF_node(bus);
> -	for (dn = busdn->child; dn; dn = dn->sibling) {
> -		pdn = PCI_DN(dn);
> -		if (pdn && pdn->devfn == devfn
> -		    && of_device_is_available(dn)) {
> -			found = true;
> -			break;
> -		}
> -	}
>  
> -	if (!found)
> -		return PCIBIOS_DEVICE_NOT_FOUND;
> +	pdn = pci_get_pdn_by_devfn(bus, devfn);

This appears to not handle the pdn == NULL case.

>  	ret = rtas_read_config(pdn, where, size, val);

But actually is handled in there ^
That would have been worth mentioning in the change log.

cheers

>  	if (*val == EEH_IO_ERROR_VALUE(size) &&
> @@ -153,23 +141,9 @@ static int rtas_pci_write_config(struct pci_bus *bus,
>  				 unsigned int devfn,
>  				 int where, int size, u32 val)
>  {
> -	struct device_node *busdn, *dn;
>  	struct pci_dn *pdn;
> -	bool found = false;
> -
> -	/* Search only direct children of the bus */
> -	busdn = pci_bus_to_OF_node(bus);
> -	for (dn = busdn->child; dn; dn = dn->sibling) {
> -		pdn = PCI_DN(dn);
> -		if (pdn && pdn->devfn == devfn
> -		    && of_device_is_available(dn)) {
> -			found = true;
> -			break;
> -		}
> -	}
>  
> -	if (!found)
> -		return PCIBIOS_DEVICE_NOT_FOUND;
> +	pdn = pci_get_pdn_by_devfn(bus, devfn);
>  
>  	return rtas_write_config(pdn, where, size, val);
>  }
> -- 
> 2.5.4 (Apple Git-61)