[Skiboot] [PATCH-RESEND] phb4: Reset FIR/NFIR registers before PHB4 probe

[Michael Neuling]

On Tue, 2018-04-03 at 08:46 +1000, Benjamin Herrenschmidt wrote:
> On Mon, 2018-04-02 at 16:12 +0530, Vaibhav Jain wrote:
> > The function phb4_probe_stack() resets "ETU Reset Register" to
> > unfreeze the PHB before it performs mmio access on the PHB. However in
> > case the FIR/NFIR registers are set while entering this function,
> > the reset of "ETU Reset Register" wont unfreeze the PHB and it will
> > remain fenced. This leads to failure during initial CRESET of the PHB
> > as mmio access is still not enabled and an error message of the form
> > below is logged:
> > 
> >  PHB#0000[0:0]: Initializing PHB4...
> >  PHB#0000[0:0]: Default system config: 0xffffffffffffffff
> >  PHB#0000[0:0]: New system config    : 0xffffffffffffffff
> >  PHB#0000[0:0]: Initial PHB CRESET is 0xffffffffffffffff
> >  PHB#0000[0:0]: Waiting for DLP PG reset to complete...
> >  <snip>
> >  PHB#0000[0:0]: Timeout waiting for DLP PG reset !
> >  PHB#0000[0:0]: Initialization failed
> > 
> > This is especially seen happening during the MPIPL flow where SBE
> > would quiesces and fence the PHB so that it doesn't stomp on the main
> > memory. However when skiboot enters phb4_probe_stack() after MPIPL,
> > the FIR/NFIR registers are set forcing PHB to re-enter fence after ETU
> > reset is done.
> > 
> > So to fix this issue the patch introduces new xscom writes to
> > phb4_probe_stack() to reset the FIR/NFIR registers before performing
> > ETU reset to enable mmio access to the PHB.
> > 
> > Signed-off-by: Vaibhav Jain <vaibhav at linux.vnet.ibm.com>
> > Tested-by: Vasant Hegde <hegdevasant at linux.vnet.ibm.com>
> 
> Acked-by: Benjamin Herrenschmidt <benh at kernel.crashing.org>
> 
> > ---
> > Change-log:
> > 
> > Resend -> Replaced xscom_read to FIR/NFIR registers with xscom_write.
> > ---
> >  hw/phb4.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/hw/phb4.c b/hw/phb4.c
> > index e45be01f..c5a33a0f 100644
> > --- a/hw/phb4.c
> > +++ b/hw/phb4.c
> > @@ -5315,6 +5315,15 @@ static void phb4_probe_stack(struct dt_nod

Can we put this in phb4_init_hw() instead or is that too late?

> > *stk_node, uint32_t pec_index,
> >  		return;
> >  	}
> >  
> > +	/* Clear errors in PFIR and NFIR */
> > +	xscom_read(gcid, pci_stack + XPEC_PCI_STK_PCI_FIR, &val);
> > +	prlog_once(PR_DEBUG, "PFIR: %llx\n", val);

Can we removed these reads and prints.  I'm not sure they add any value.

> > +	xscom_write(gcid, pci_stack + XPEC_PCI_STK_PCI_FIR, 0);
> > +
> > +	xscom_read(gcid, nest_stack + XPEC_NEST_STK_PCI_NFIR, &val);
> > +	prlog_once(PR_DEBUG, "NFIR: %llx\n", val);
> > +	xscom_write(gcid, nest_stack + XPEC_NEST_STK_PCI_NFIR, 0);
> > +
> >  	/* Check ETU reset */
> >  	xscom_read(gcid, pci_stack + XPEC_PCI_STK_ETU_RESET, &val);
> >  	prlog_once(PR_DEBUG, "ETU reset: %llx\n", val);
> 
>