[Skiboot] [PATCH v3] hw/xscom: Reset XSCOM engine after finite number of retries when busy

[Vaidyanathan Srinivasan]

* Mahesh J Salgaonkar <mahesh at linux.vnet.ibm.com> [2016-06-03 11:16:30]:

> On 2016-05-31 17:09:01 Tue, Vipin K Parashar wrote:

[snip]

> I understand that today we are not sure about how to verify whether xscom reset
> has succeeded or not and hence this patch adds the delay. But I think we
> should add FIXME comment here so that in future we can fix this
> properly by talking to hardware folks to find out alternate way to
> verify xscom reset completion status.

Yes, we could add FIXME in comment until we can figure out the exact
cycle checks from hardware team. But this is a very very rare recovery
case/scenario and should not hold-up OPAL under normal operation.
 
> > +			ts.tv_sec = 0;
> > +			ts.tv_nsec = 10 * 1000;
> > +			nanosleep_nopoll(&ts, NULL);
> > +		}
> > +
> > +		/* Log error if we have retried enough and its still busy */
> > +		if (retries == XSCOM_BUSY_MAX_RETRIES)
> > +			log_simple_error(&e_info(OPAL_RC_XSCOM_BUSY),
> > +				"XSCOM: %s-busy error gcid=0x%x pcb_addr=0x%x "
> > +				"stat=0x%x\n", is_write ? "write" : "read",
> > +				gcid, pcb_addr, stat);
> >  		return OPAL_BUSY;
> > +
> >  	/* CPU is asleep, don't retry */
> >  	case 2:
> >  		return OPAL_WRONG_STATE;
> > @@ -177,15 +210,16 @@ static bool xscom_gcid_ok(uint32_t gcid)
> >   */
> >  static int __xscom_read(uint32_t gcid, uint32_t pcb_addr, uint64_t *val)
> >  {
> > +	int i;
> >  	uint64_t hmer;
> > -	int64_t ret;
> > +	int64_t ret, retries = 0;
> >  
> >  	if (!xscom_gcid_ok(gcid)) {
> >  		prerror("%s: invalid XSCOM gcid 0x%x\n", __func__, gcid);
> >  		return OPAL_PARAMETER;
> >  	}
> >  
> > -	for (;;) {
> > +	for (i = 0; i <= XSCOM_BUSY_MAX_RETRIES; i++) {

        for (retries = 0; retries <= XSCOM_BUSY_MAX_RETRIES; retries++)

> >  		/* Clear status bits in HMER (HMER is special
> >  		 * writing to it *ands* bits
> >  		 */
> > @@ -199,27 +233,31 @@ static int __xscom_read(uint32_t gcid, uint32_t pcb_addr, uint64_t *val)
> >  
> >  		/* Check for error */
> >  		if (!(hmer & SPR_HMER_XSCOM_FAIL))
> > -			break;
> > +			return OPAL_SUCCESS;
> >  
> >  		/* Handle error and possibly eventually retry */
> > -		ret = xscom_handle_error(hmer, gcid, pcb_addr, false);
> > -		if (ret == OPAL_HARDWARE || ret == OPAL_WRONG_STATE)
> > -			return ret;
> > +		ret = xscom_handle_error(hmer, gcid, pcb_addr, false, retries);
> > +		if (ret != OPAL_BUSY)
> > +			break;
> > +		retries++;
> 
> I think we can get rid off extra variable here 'retries'. Just pass
> variable 'i' to xscom_handle_error() which anyway gets incremented as
> part of for loop.

Good idea. How about keep the name retries, and remove i :)

--Vaidy