[v7,39/50] powerpc/powernv: Fundamental reset in pnv_pci_reset_secondary_bus()

In pnv_pci_reset_secondary_bus(), we should issue fundamental
reset if any one subordinate device of the specified is requesting
that. Otherwise, the device might not come up after the reset.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/eeh-powernv.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

On Thu, Nov 05, 2015 at 12:12:39AM +1100, Gavin Shan wrote:
>In pnv_pci_reset_secondary_bus(), we should issue fundamental
>reset if any one subordinate device of the specified is requesting
                                        ^^^^^^^^^^^^^^

                                        the specified bus

I put the note reminding me to admend the changelog in next revision.

>that. Otherwise, the device might not come up after the reset.
>
>Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>---
> arch/powerpc/platforms/powernv/eeh-powernv.c | 21 ++++++++++++++++++++-
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
>diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
>index c69b6a1..ab8b93e 100644
>--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
>+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
>@@ -878,9 +878,28 @@ static int pnv_eeh_bridge_reset(struct pci_dev *dev, int option)
> 	return 0;
> }
>
>+static int pnv_pci_dev_reset_type(struct pci_dev *pdev, void *data)
>+{
>+	int *freset = data;
>+
>+	/*
>+	 * Stop the iteration immediately if there has any one
>+	 * PCI device requesting fundamental reset.
>+	 */
>+	*freset |= pdev->needs_freset;
>+	return *freset;
>+}
>+
> void pnv_pci_reset_secondary_bus(struct pci_dev *dev)
> {
>-	pnv_eeh_bridge_reset(dev, EEH_RESET_HOT);
>+	int option, freset = 0;
>+
>+	if (dev->subordinate)
>+		pci_walk_bus(dev->subordinate,
>+			     pnv_pci_dev_reset_type, &freset);
>+
>+	option = freset ? EEH_RESET_FUNDAMENTAL : EEH_RESET_HOT;
>+	pnv_eeh_bridge_reset(dev, option);
> 	pnv_eeh_bridge_reset(dev, EEH_RESET_DEACTIVATE);
> }
>
>-- 
>2.1.0
>

Gavin Shan <gwshan@linux.vnet.ibm.com> writes:

>  void pnv_pci_reset_secondary_bus(struct pci_dev *dev)
>  {
> -	pnv_eeh_bridge_reset(dev, EEH_RESET_HOT);
> +	int option, freset = 0;
> +
> +	if (dev->subordinate)
> +		pci_walk_bus(dev->subordinate,
> +			     pnv_pci_dev_reset_type, &freset);
> +
> +	option = freset ? EEH_RESET_FUNDAMENTAL : EEH_RESET_HOT;
> +	pnv_eeh_bridge_reset(dev, option);

According to the skiboot sources, fundamental reset isn't supported on
p5ioc2. As far as I can tell from your corresponding skiboot patches,
this is still the case after they are applied. Do we need a fallback to
EEH_RESET_HOT in this case? Otherwise there will be no reset performed
at all.

Likewise, if the FUNDAMENTAL reset fails for any reason, should we fall
back to a HOT reset?

Regards,
Daniel

On Fri, Nov 13, 2015 at 11:08:29AM +1100, Daniel Axtens wrote:
>Gavin Shan <gwshan@linux.vnet.ibm.com> writes:
>
>>  void pnv_pci_reset_secondary_bus(struct pci_dev *dev)
>>  {
>> -	pnv_eeh_bridge_reset(dev, EEH_RESET_HOT);
>> +	int option, freset = 0;
>> +
>> +	if (dev->subordinate)
>> +		pci_walk_bus(dev->subordinate,
>> +			     pnv_pci_dev_reset_type, &freset);
>> +
>> +	option = freset ? EEH_RESET_FUNDAMENTAL : EEH_RESET_HOT;
>> +	pnv_eeh_bridge_reset(dev, option);
>
>According to the skiboot sources, fundamental reset isn't supported on
>p5ioc2. As far as I can tell from your corresponding skiboot patches,
>this is still the case after they are applied. Do we need a fallback to
>EEH_RESET_HOT in this case? Otherwise there will be no reset performed
>at all.
>
>Likewise, if the FUNDAMENTAL reset fails for any reason, should we fall
>back to a HOT reset?
>

P5IOC2 won't export any PCI slots. So kernel won't issue fundamental reset
to PCI buses on P5IOC2.

We had the failback: hot reset is picked if fundamental reset can't be
supported on the target PCI bus. In case fundamental reset fails, we
shouldn't go ahead try hot reset.

Thanks,
Gavin

>Regards,
>Daniel

On Fri, 2015-11-13 at 11:08 +1100, Daniel Axtens wrote:
> Gavin Shan <gwshan@linux.vnet.ibm.com> writes:
> 
> >  void pnv_pci_reset_secondary_bus(struct pci_dev *dev)
> >  {
> > -> > 	> > pnv_eeh_bridge_reset(dev, EEH_RESET_HOT);
> > +> > 	> > int option, freset = 0;
> > +
> > +> > 	> > if (dev->subordinate)
> > +> > 	> > 	> > pci_walk_bus(dev->subordinate,
> > +> > 	> > 	> > 	> >      pnv_pci_dev_reset_type, &freset);
> > +
> > +> > 	> > option = freset ? EEH_RESET_FUNDAMENTAL : EEH_RESET_HOT;
> > +> > 	> > pnv_eeh_bridge_reset(dev, option);
> 
> According to the skiboot sources, fundamental reset isn't supported on
> p5ioc2. As far as I can tell from your corresponding skiboot patches,
> this is still the case after they are applied. Do we need a fallback to
> EEH_RESET_HOT in this case? Otherwise there will be no reset performed
> at all.

We don't really care that much about what happens on p5ioc2 :-)

> Likewise, if the FUNDAMENTAL reset fails for any reason, should we fall
> back to a HOT reset?

Probably.

Cheers,
Ben.

Following some discussion on IRC, it looks like there are roughly 2
machines on the planet with skiboot and p5ioc2, so I'm not worried about
that any more.

I am still vaguely concerned about a failing fundamental reset.

Regards,
Daniel