| Message ID | 20180726103942.28347-1-npiggin@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | libpdbg/p9chip: use SPWKUP_FSP instead of SPWKUP_OTR | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | master/apply_patch Successfully applied |
| snowpatch_ozlabs/build-multiarch | success | Test build-multiarch on branch master |
On 07/26/2018 04:09 PM, Nicholas Piggin wrote: > Dean Sanner notes that pdbg should not use the OTR special wakeup > register, as it is used by the PM complex (i.e., SGPE, CME, PGPE). > It should use the FSP register instead, which is reserved for service > processor firmware, and nothing else in OpenBMC uses this at the > moment. > > In theory when pdbg is used via the host, it should be using the HYP > register via some OPAL arbitration mechanism (OPAL being the owner of > the HYP register), but in the absence of any of that mechanism, FSP > is the best option. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > libpdbg/p9chip.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/libpdbg/p9chip.c b/libpdbg/p9chip.c > index c5de3bb..3a7ff8a 100644 > --- a/libpdbg/p9chip.c > +++ b/libpdbg/p9chip.c > @@ -71,7 +71,7 @@ > #define NET_CTRL0_FENCE_EN PPC_BIT(18) > #define NET_CTRL0_WOR 0xf0042 > #define PPM_GPMMR 0xf0100 > -#define PPM_SPWKUP_OTR 0xf010a > +#define PPM_SPWKUP_FSP 0xf010b > #define PPM_SSHOTR 0xf0113 Now that we are using FSP register to assert special wakeup, shouldn't we also change above to 0xf0111 (STOP_STATE_HIST_FSP_REG) for special wakeup DONE bit check ? Thanks, -Mahesh. > #define SPECIAL_WKUP_DONE PPC_BIT(1) > > @@ -468,7 +468,7 @@ static int p9_core_probe(struct pdbg_target *target) > if (!(value & NET_CTRL0_CHIPLET_ENABLE)) > return -1; > > - CHECK_ERR(pib_write(target, PPM_SPWKUP_OTR, PPC_BIT(0))); > + CHECK_ERR(pib_write(target, PPM_SPWKUP_FSP, PPC_BIT(0))); > do { > usleep(1000); > CHECK_ERR(pib_read(target, PPM_SSHOTR, &value)); > @@ -486,7 +486,7 @@ static int p9_core_probe(struct pdbg_target *target) > static void p9_core_release(struct pdbg_target *target) > { > usleep(1); /* enforce small delay before and after it is cleared */ > - pib_write(target, PPM_SPWKUP_OTR, 0); > + pib_write(target, PPM_SPWKUP_FSP, 0); > usleep(10000); > } >
On Mon, 30 Jul 2018 10:39:50 +0530 Mahesh Jagannath Salgaonkar <mahesh@linux.vnet.ibm.com> wrote: > On 07/26/2018 04:09 PM, Nicholas Piggin wrote: > > Dean Sanner notes that pdbg should not use the OTR special wakeup > > register, as it is used by the PM complex (i.e., SGPE, CME, PGPE). > > It should use the FSP register instead, which is reserved for service > > processor firmware, and nothing else in OpenBMC uses this at the > > moment. > > > > In theory when pdbg is used via the host, it should be using the HYP > > register via some OPAL arbitration mechanism (OPAL being the owner of > > the HYP register), but in the absence of any of that mechanism, FSP > > is the best option. > > > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > --- > > libpdbg/p9chip.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/libpdbg/p9chip.c b/libpdbg/p9chip.c > > index c5de3bb..3a7ff8a 100644 > > --- a/libpdbg/p9chip.c > > +++ b/libpdbg/p9chip.c > > @@ -71,7 +71,7 @@ > > #define NET_CTRL0_FENCE_EN PPC_BIT(18) > > #define NET_CTRL0_WOR 0xf0042 > > #define PPM_GPMMR 0xf0100 > > -#define PPM_SPWKUP_OTR 0xf010a > > +#define PPM_SPWKUP_FSP 0xf010b > > > #define PPM_SSHOTR 0xf0113 > > Now that we are using FSP register to assert special wakeup, shouldn't > we also change above to 0xf0111 (STOP_STATE_HIST_FSP_REG) for special > wakeup DONE bit check ? Oh you're right, hmm I must have messed up my testing. I'll resubmit. Thanks, Nick
diff --git a/libpdbg/p9chip.c b/libpdbg/p9chip.c index c5de3bb..3a7ff8a 100644 --- a/libpdbg/p9chip.c +++ b/libpdbg/p9chip.c @@ -71,7 +71,7 @@ #define NET_CTRL0_FENCE_EN PPC_BIT(18) #define NET_CTRL0_WOR 0xf0042 #define PPM_GPMMR 0xf0100 -#define PPM_SPWKUP_OTR 0xf010a +#define PPM_SPWKUP_FSP 0xf010b #define PPM_SSHOTR 0xf0113 #define SPECIAL_WKUP_DONE PPC_BIT(1) @@ -468,7 +468,7 @@ static int p9_core_probe(struct pdbg_target *target) if (!(value & NET_CTRL0_CHIPLET_ENABLE)) return -1; - CHECK_ERR(pib_write(target, PPM_SPWKUP_OTR, PPC_BIT(0))); + CHECK_ERR(pib_write(target, PPM_SPWKUP_FSP, PPC_BIT(0))); do { usleep(1000); CHECK_ERR(pib_read(target, PPM_SSHOTR, &value)); @@ -486,7 +486,7 @@ static int p9_core_probe(struct pdbg_target *target) static void p9_core_release(struct pdbg_target *target) { usleep(1); /* enforce small delay before and after it is cleared */ - pib_write(target, PPM_SPWKUP_OTR, 0); + pib_write(target, PPM_SPWKUP_FSP, 0); usleep(10000); }
Dean Sanner notes that pdbg should not use the OTR special wakeup register, as it is used by the PM complex (i.e., SGPE, CME, PGPE). It should use the FSP register instead, which is reserved for service processor firmware, and nothing else in OpenBMC uses this at the moment. In theory when pdbg is used via the host, it should be using the HYP register via some OPAL arbitration mechanism (OPAL being the owner of the HYP register), but in the absence of any of that mechanism, FSP is the best option. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- libpdbg/p9chip.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)