Re: [PATCH v4 10/21] powerpc/powernv: Fundamental reset for PCI bus reset

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 05/11/2015 04:47 PM, Gavin Shan wrote:
> On Sun, May 10, 2015 at 12:12:18AM +1000, Alexey Kardashevskiy wrote:
>> On 05/01/2015 04:02 PM, Gavin Shan wrote:
>>> Function pnv_pci_reset_secondary_bus() is used to reset specified
>>> PCI bus, which is leaded by root complex or PCI bridge. That means
>>> the function shouldn't be called on PCI root bus and the patch
>>> removes the logic for that case.
>>>
>>> Also, some adapters beneath the indicated PCI bus may require
>>> fundamental reset in order to successfully reload their firmwares
>>> after the reset. The patch translates hot reset to fundamental reset
>>> for that case.
>>>
>>> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>>> ---
>>>   arch/powerpc/platforms/powernv/eeh-powernv.c | 35 +++++++++++++++++++++-------
>>>   1 file changed, 26 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
>>> index 3c01095..58e4dcf 100644
>>> --- a/arch/powerpc/platforms/powernv/eeh-powernv.c
>>> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
>>> @@ -888,18 +888,35 @@ static int pnv_eeh_bridge_reset(struct pci_dev *dev, int option)
>>>   	return (rc == OPAL_SUCCESS) ? 0 : -EIO;
>>>   }
>>>
>>> -void pnv_pci_reset_secondary_bus(struct pci_dev *dev)
>>
>>
>> Why changing dev to pdev? Keeping "dev" could make the patch simpler.
>>
>
> In the early stage when I wrote the EEH code, I had "dev" to refer PCI
> device, which isn't precisely enough. Actually, "dev" means "struct device"
> while "pdev" stands for "struct pci_dev". That's why I changed it.


The rest of the file and the kernel overall use "dev" for pci_dev just 
fine. I would not bother.


>>> +static int pnv_pci_dev_reset_type(struct pci_dev *pdev, void *data)
>>>   {
>>> -	struct pci_controller *hose;
>>> +	int *freset = data;
>>>
>>> -	if (pci_is_root_bus(dev->bus)) {
>>> -		hose = pci_bus_to_host(dev->bus);
>>> -		pnv_eeh_phb_reset(hose, EEH_RESET_HOT);
>>> -		pnv_eeh_phb_reset(hose, EEH_RESET_DEACTIVATE);
>>> -	} else {
>>> -		pnv_eeh_bridge_reset(dev, EEH_RESET_HOT);
>>> -		pnv_eeh_bridge_reset(dev, EEH_RESET_DEACTIVATE);
>>> +	/*
>>> +	 * Stop the iteration immediately if there is any
>>> +	 * one PCI device requesting fundamental reset
>>> +	 */
>>> +	*freset |= pdev->needs_freset;
>>> +	return *freset;
>>> +}
>>> +
>>> +void pnv_pci_reset_secondary_bus(struct pci_dev *pdev)
>>> +{
>>> +	int option = EEH_RESET_HOT;
>>> +	int freset = 0;
>>> +
>>> +	/* Check if there're any PCI devices asking for fundamental reset */
>>> +	if (pdev->subordinate) {
>>> +		pci_walk_bus(pdev->subordinate,
>>> +			     pnv_pci_dev_reset_type,
>>> +			     &freset);
>>> +		if (freset)
>>> +			option = EEH_RESET_FUNDAMENTAL;
>>>   	}
>>> +
>>> +	/* Issue the requested type of reset */
>>> +	pnv_eeh_bridge_reset(pdev, option);
>>> +	pnv_eeh_bridge_reset(pdev, EEH_RESET_DEACTIVATE);
>>>   }
>>>
>>>   /**
>>>
>
> Thanks,
> Gavin
>


-- 
Alexey