Re: [PATCH 1/1] powerpc: Add powerpc PCI-E reset API implementation

[linas@austin.ibm.com (Linas Vepstas)]

On Fri, Apr 06, 2007 at 09:07:04AM -0500, Brian King wrote:
> 
> This patch requires a generic PCI layer patch, which is in Greg's
> queue for 2.6.22, 

I managed to miss seeing that patch, even though I thought I was 
subscribed to the mailing list. Was there any discusion of it?

> +int pcibios_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state)

I think you'll need an EXPORT_SMBOL_GPL if you plan to call this from
a module.

> +	switch (state) {
> +	case pci_reset_normal:
> +		rtas_pci_slot_reset(pdn, 0);

I find this naming confusing. rtas_pci_slot_reset(pdn, 0),
for PCI ad PCI-X means "deassert the reset"; it does not 
mean "do a normal reset".

> +		break;
> +	case pci_reset_pcie_hot_reset:
> +		rtas_pci_slot_reset(pdn, 1);
> +		break;
> +	case pci_reset_pcie_warm_reset:
> +		rtas_pci_slot_reset(pdn, 3);

For PCI and PCI-X, rtas_pci_slot_reset(pdn, 1) means "hold the
reset line high, until such time that its de-asserted."

> +	return 0;

I notice that you do no error checking. I recently wrapped 
rtas_set_slot_reset() to wait for slot status to settle down 
before reporting success or failure of the reset. 

Although the PAPR maps 1 to hot reset, and 3 to #PERST, I always
had the impression that they managed to reverse meaing of these two
(i.e. its a poor match to what the PCI-E spec says), and I never 
understood why. I am still thinking that the correct reset seqeunce
on linux is to try "3" first, if its supported, and then try a "1".
I've not taken steps to do this, though.

I could wrap rtas_set_slot_reset()  to try a "3" first, for 
PCI-E slots, and do 1 "1" only if that fails. Would this solve 
the problem that you are having?

--linas