hw/npu2: Merge reset functions
diff mbox series

Message ID 20190322054523.29670-1-oohall@gmail.com
State New
Headers show
Series
  • hw/npu2: Merge reset functions
Related show

Checks

Context Check Description
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco success Signed-off-by present
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot fail Test snowpatch/job/snowpatch-skiboot on branch master
snowpatch_ozlabs/apply_patch success Successfully applied on branch master (b392d785eb49630b9f00fef8d17944ed82b2c1fe)

Commit Message

Oliver O'Halloran March 22, 2019, 5:45 a.m. UTC
Seems like we should be doing the same cache purge operation in the
CRESET case, and we don't even implement FRESET.

Cc: Reza Arbab <arbab@linux.ibm.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
 hw/npu2.c | 27 +++++++++------------------
 1 file changed, 9 insertions(+), 18 deletions(-)

Comments

Reza Arbab March 22, 2019, 9:15 p.m. UTC | #1
On Fri, Mar 22, 2019 at 04:45:23PM +1100, Oliver O'Halloran wrote:
> static int64_t npu2_freset(struct pci_slot *slot __unused)
> {
>-	return OPAL_SUCCESS;
>+	return npu2_reset("Freset");
> }

IIRC, there actually is a reason npu2_freset() doesn't do anything.

Alistair, do you recall?
Frederic Barrat March 25, 2019, 10:12 a.m. UTC | #2
Le 22/03/2019 à 22:15, Reza Arbab a écrit :
> On Fri, Mar 22, 2019 at 04:45:23PM +1100, Oliver O'Halloran wrote:
>> static int64_t npu2_freset(struct pci_slot *slot __unused)
>> {
>> -    return OPAL_SUCCESS;
>> +    return npu2_reset("Freset");
>> }
> 
> IIRC, there actually is a reason npu2_freset() doesn't do anything.
> 
> Alistair, do you recall?
> 

I'm certainly not trying to fill-in for Alistair ;-) but freset is 
called when the PCI slots are init when skiboot starts, typically to 
kick-in link training. Except for nvlink. So here we'd be resetting the 
NTL instead of doing nothing. Any chance it could have consequences?

   Fred
Alistair Popple March 26, 2019, 2:15 a.m. UTC | #3
On Monday, 25 March 2019 11:12:49 AM AEDT Frederic Barrat wrote:
> Le 22/03/2019 à 22:15, Reza Arbab a écrit :
> > On Fri, Mar 22, 2019 at 04:45:23PM +1100, Oliver O'Halloran wrote:
> >> static int64_t npu2_freset(struct pci_slot *slot __unused)
> >> {
> >> -    return OPAL_SUCCESS;
> >> +    return npu2_reset("Freset");
> >> }
> > 
> > IIRC, there actually is a reason npu2_freset() doesn't do anything.
> > 
> > Alistair, do you recall?
> 
> I'm certainly not trying to fill-in for Alistair ;-) but freset is
> called when the PCI slots are init when skiboot starts, typically to
> kick-in link training. Except for nvlink. So here we'd be resetting the
> NTL instead of doing nothing. Any chance it could have consequences?

I don't recall there being any reason why freset doesn't do anything other 
than it doesn't need to do anything.

The NVLink resets were originally used for resetting the NVLink HW to a POR 
state so the driver could retrain links, etc. in the case of a GPU reset. For 
coherent memory we required some work arounds to fence the NVLink as well once 
the link was brought down. Calling this from freset() shouldn't cause any 
issues during skiboot start, although I am not sure what the argument for 
doing it is either as unlike PCIe (and perhaps CAPI?) we can't do nvlink 
training from Skiboot.

- Alistair

>    Fred

Patch
diff mbox series

diff --git a/hw/npu2.c b/hw/npu2.c
index 4ecc91350e82..a630536ca418 100644
--- a/hw/npu2.c
+++ b/hw/npu2.c
@@ -1200,14 +1200,14 @@  static int64_t npu2_get_power_state(struct pci_slot *slot __unused, uint8_t *val
 	return OPAL_SUCCESS;
 }
 
-static int64_t npu2_hreset(struct pci_slot *slot __unused)
+static int64_t npu2_reset(const char *reset_type)
 {
 	struct npu2 *p;
 	int i;
 	struct npu2_dev *ndev;
 
 	p = phb_to_npu2_nvlink(slot->phb);
-	NPU2INF(p, "Hreset PHB state\n");
+	NPU2INF(p, "%s PHB state\n", reset_type);
 
 	for (i = 0; i < p->total_devices; i++) {
 		ndev = &p->devices[i];
@@ -1219,28 +1219,19 @@  static int64_t npu2_hreset(struct pci_slot *slot __unused)
 	return purge_l2_l3_caches();
 }
 
+static int64_t npu2_hreset(struct pci_slot *slot __unused)
+{
+	return npu2_reset("Hreset");
+}
+
 static int64_t npu2_freset(struct pci_slot *slot __unused)
 {
-	return OPAL_SUCCESS;
+	return npu2_reset("Freset");
 }
 
 static int64_t npu2_creset(struct pci_slot *slot)
 {
-	struct npu2 *p;
-	int i;
-	struct npu2_dev *ndev;
-
-	p = phb_to_npu2_nvlink(slot->phb);
-	NPU2INF(p, "Creset PHB state\n");
-
-	for (i = 0; i < p->total_devices; i++) {
-		ndev = &p->devices[i];
-		if (ndev) {
-			NPU2DEVINF(ndev, "Resetting device\n");
-			reset_ntl(ndev);
-		}
-	}
-	return OPAL_SUCCESS;
+	return npu2_reset("Creset");
 }
 
 static struct pci_slot *npu2_slot_create(struct phb *phb)