[Skiboot] [PATCH 4/5] xscom: Add indirect form 1 scoms

[Michael Neuling]

On Wed, 2017-03-22 at 16:29 +0100, Cédric Le Goater wrote:
> On 03/22/2017 03:59 AM, Michael Neuling wrote:
> > Add code to perform indirect form 1 scoms.
> > 
> > POWER8 does form 0 only. POWER9 adds form 1. The form is determined
> > from the address only. Hardware only allows writes for form 1.
> > 
> > Only hostboot uses these scoms during IPL, so they are unused by
> > skiboot currently.
> > 
> > Signed-off-by: Michael Neuling <mikey at neuling.org>
> > ---
> >  hw/xscom.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 46 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/xscom.c b/hw/xscom.c
> > index 1add658776..7f3e329fee 100644
> > --- a/hw/xscom.c
> > +++ b/hw/xscom.c
> > @@ -317,7 +317,8 @@ static int __xscom_write(uint32_t gcid, uint32_t
> > pcb_addr, uint64_t val)
> >  /*
> >   * Indirect XSCOM access functions
> >   */
> > -static int xscom_indirect_read(uint32_t gcid, uint64_t pcb_addr, uint64_t
> > *val)
> > +static int xscom_indirect_read_form0(uint32_t gcid, uint64_t pcb_addr,
> > +				     uint64_t *val)
> >  {
> >  	uint32_t addr;
> >  	uint64_t data;
> > @@ -360,7 +361,18 @@ static int xscom_indirect_read(uint32_t gcid, uint64_t
> > pcb_addr, uint64_t *val)
> >  	return rc;
> >  }
> > 
> > -static int xscom_indirect_write(uint32_t gcid, uint64_t pcb_addr, uint64_t
> > val)
> > +static int xscom_indirect_read(uint32_t gcid, uint64_t pcb_addr, uint64_t
> > *val)
> > +{
> > +	uint64_t form = (pcb_addr >> 60) & 1;
> > +
> > +	if ((proc_gen == proc_gen_p9) && (form == 1))
> > +		return OPAL_UNSUPPORTED;
> > +
> > +	return xscom_indirect_read_form0(gcid, pcb_addr, val);
> > +}
> > +
> > +static int xscom_indirect_write_form0(uint32_t gcid, uint64_t pcb_addr,
> > +				      uint64_t val)
> >  {
> >  	uint32_t addr;
> >  	uint64_t data;
> > @@ -402,6 +414,38 @@ static int xscom_indirect_write(uint32_t gcid, uint64_t
> > pcb_addr, uint64_t val)
> >  	return rc;
> >  }
> > 
> > +static int xscom_indirect_write_form1(uint32_t gcid, uint64_t pcb_addr,
> > +				      uint64_t val)
> > +{
> > +	uint32_t addr;
> > +	uint64_t data;
> > +	int rc;
> > +
> > +	if (proc_gen < proc_gen_p9)
> > +		return OPAL_UNSUPPORTED;
> > +	if (val & 0xfff0000000000000ULL)
> > +		return OPAL_PARAMETER;
> > +
> > +	/* Mangle address and data for form1 */
> > +	addr = (pcb_addr & 0x000ffffffff);
> > +	data = (pcb_addr & 0xfff00000000) << 20;
> 
> some bitmask definitions would help.

I added one for the val check but...

I'm leaving the pcb_addr inline mangling as IMHO it's easier to read as is.  It
clearly shows how the pcb_addr is split and placed in addr and data. 
Abstracting it with bit defs will make it harder to see what's happening.

> > +	data |= val;
> > +	rc = __xscom_write(gcid, addr, data);
> > +	if (rc)
> > +		val = (uint64_t)-1;
> 
> who cares about val ? 

Good point. I'll remove

> > +	return rc;
> > +}
> > +
> > +static int xscom_indirect_write(uint32_t gcid, uint64_t pcb_addr, uint64_t
> > val)
> > +{
> > +	uint64_t form = (pcb_addr >> 60) & 1;
> 
> maybe use a helper ? 

OK, I'll add.

Thanks

Mikey