diff mbox

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

Message ID 1446642770-4681-40-git-send-email-gwshan@linux.vnet.ibm.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Gavin Shan Nov. 4, 2015, 1:12 p.m. UTC
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(-)

Comments

Gavin Shan Nov. 12, 2015, 6:15 a.m. UTC | #1
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
>

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Axtens Nov. 13, 2015, 12:08 a.m. UTC | #2
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
Gavin Shan Nov. 13, 2015, 12:20 a.m. UTC | #3
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


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Benjamin Herrenschmidt Nov. 13, 2015, 12:23 a.m. UTC | #4
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Axtens Nov. 13, 2015, 12:23 a.m. UTC | #5
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
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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);
 }