Subject: Re: [PATCH v7 38/50] powerpc/powernv: Exclude root bus in pnv_pci_reset_secondary_bus()

From: Gavin Shan <[email-masked]>
Date: Fri, 13 Nov 2015 10:25:50 +1100

On Fri, Nov 13, 2015 at 09:59:27AM +1100, Daniel Axtens wrote:
>Gavin Shan <[email-masked]> writes:
>
>> When pnv_pci_reset_secondary_bus() is called to issue reset on
>> the indicated secondary bus, the bus can't be root bus. So we
>> needn't consider root bus in the function.
>
>It took me a while to convince myself that this is correct. For the
>record, this is why it's correct:
>
>pnv_pci_reset_secondary_bus fills the reset_secondary_bus callback in
>the pci_controller_ops structure, and isn't used elsewhere.
>
>In arch/powerpc/kernel/pci.c, that callback is called (if it exists) in 
>pcibios_reset_secondary_bus(). It's not called anywhere else.
>
>The PPC pcibios_reset_secondary_bus overrides the weak version in
>drivers/pci/pci.c. It's called from the same file by
>pci_reset_bridge_secondary_device() (and nowhere else).
>
>pci_reset_bridge_secondary_device() is nicely documented:
>
>/**
> * pci_reset_bridge_secondary_bus - Reset the secondary bus on a PCI bridge.
> * @dev: Bridge device
> *
> * Use the bridge control register to assert reset on the secondary bus.
> * Devices on the secondary bus are left in power-on state.
> */
>
>Therefore, by the definiton of pci_reset_bridge_secondary_bus,
>pnv_pci_reset_secondary_bus() can only be called with a bridge
>device. As such, a bridge reset only is appropriate. If this breaks
>anything, the caller is broken.
>
>It might be worth including a condensed version of this in the commit
>message.
>

Right. I'll add more description to the changelog in next revision.

>Reviewed-by: Daniel Axtens <[email-masked]>
>

Thanks,
Gavin

>Regards,
>Daniel Axtens
>
>>
>> Signed-off-by: Gavin Shan <[email-masked]>
>> ---
>>  arch/powerpc/platforms/powernv/eeh-powernv.c | 12 ++----------
>>  1 file changed, 2 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
>> index a7d84a4..c69b6a1 100644
>> --- a/arch/powerpc/platforms/powernv/eeh-powernv.c
>> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
>> @@ -880,16 +880,8 @@ static int pnv_eeh_bridge_reset(struct pci_dev *dev, int option)
>>  
>>  void pnv_pci_reset_secondary_bus(struct pci_dev *dev)
>>  {
>> -	struct pci_controller *hose;
>> -
>> -	if (pci_is_root_bus(dev->bus)) {
>> -		hose = pci_bus_to_host(dev->bus);
>> -		pnv_eeh_root_reset(hose, EEH_RESET_HOT);
>> -		pnv_eeh_root_reset(hose, EEH_RESET_DEACTIVATE);
>> -	} else {
>> -		pnv_eeh_bridge_reset(dev, EEH_RESET_HOT);
>> -		pnv_eeh_bridge_reset(dev, EEH_RESET_DEACTIVATE);
>> -	}
>> +	pnv_eeh_bridge_reset(dev, EEH_RESET_HOT);
>> +	pnv_eeh_bridge_reset(dev, EEH_RESET_DEACTIVATE);
>>  }
>>  
>>  /**
>> -- 
>> 2.1.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to [email-masked]
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html