[Skiboot] [PATCH 1/2] opal/xscom: Move the delay inside xscom_reset() function.

[Nicholas Piggin]

On Fri, 8 Dec 2017 11:13:52 +0530
Mahesh Jagannath Salgaonkar <mahesh at linux.vnet.ibm.com> wrote:

> On 12/08/2017 09:12 AM, Nicholas Piggin wrote:
> > On Thu, 07 Dec 2017 21:43:00 +0530
> > Mahesh J Salgaonkar <mahesh at linux.vnet.ibm.com> wrote:
> >   
> >> From: Mahesh Salgaonkar <mahesh at linux.vnet.ibm.com>
> >>
> >> So caller of xscom_reset() does not have to bother about adding a delay
> >> separately.  
> > 
> > Hmm, this does change behaviour a bit. I suppose it is harmless because
> > it's just adding small delays in error paths. But it seems that the delay
> > was only needed for busy xscom, and only to delay the retry of the same
> > xscom. Whereas xscom_reset gets called in more situations.
> > 
> > Does the xscom_clear_error in your next patch also require some delays?  
> 
> Yes, it does. I struggled for couple of days to get invalid address for
> clearing write to 20010800 immediately after xscom_reset(). Without this
> delay the write was responding with scom done with no error, which was
> completely confusing as there is no register at 0x20010800. With the
> delay things started working as expected. It looks like delay is very
> much required if we are doing next scom operation immediately after the
> reset.

Okay that makes sense, I was just curious. Might be good to comment that
as well in the clear_error path.

> > I wouldn't mind having those delays explicitly commented in there.  
> 
> I see your point. If there is no scom operation immediately after the
> reset we can do away with the delay. In the normal scom failure other
> than busy scom and clear scom we don;t need this delay. How about adding
> 'need_delay' as a second argument to the xscom_reset(gcid,
> need_delay=<true|false>) function to control the delay instead of adding
> delays multiple places explicitly ? Do you think that should should work
> ? I will respin v2 with the change if it's ok.

Yeah that could be better. Although be careful in case the caller does
another xscom_read/write immediately after a failure. If that is a
concern I would err on the side of caution.

I don't have a better alternative, so long as you are happy with it,
that's fine by me.

Thanks,
Nick