fwochi.c and bus_space_barrier()

[Sean Bruno]

On Thu, 2009-04-16 at 22:20 +0200, Andreas Tobler wrote:
> Sean Bruno wrote:
> >>> You may want to retry several times.  Like you pointed out in earlier
> >>> posts, this issue seems to be a race condition.
> >> Heh, now I remember, I did not speak about a race condition, but about a 
> >> timing issue.
> >>
> >> If I leave the printfs away, it panics here.
> >>
> >> for (lps = 0, lps_counter = 0; !lps && lps_counter < 3; lps_counter++) {
> >>                  lps = (OREAD(sc, OHCI_HCCCTL) & OHCI_HCC_LPS);
> >>                  if (!lps) {
> >>                          pause("fwlps", (50 * hz + 999) / 1000);
> >>                          device_printf(dev, "lps not set, 
> >> attempt(%d)\n", lps_cou
> >> nter);
> >>                  } /* else
> >>                          device_printf(dev, "lps(%0x) set\n", lps);*/
> >>          }
> >>
> >> In my case the lps is not NULL, so we print something in the first run 
> >> of the loop, this print statement is enough 'time' for the card to come 
> >> up. If we leave the printf away, it is not enough time to come up for 
> >> the card. Panic.
> >>
> >> This was the same thing I reported, adding a printf statement at the 
> >> beginning of fwphy_rddata cures my panic.
> >>
> >> So I'd suggest to leave the lps test away and add always a pause(9), or 
> >> does this cause headache on other archs?
> >>
> >> Thanks,
> >> Andreas
> >>
> > 
> > 
> > Ok, I think I've finally caught up to Marius (at least in this
> > situation).  
> > 
> > The *ACTUAL* issue is that fwochi_probe_phy() code isn't properly
> > handling the transition state from LPS==0 to LPS==1.  In this period of
> > time, the internal SCLK on the firewire board may have not started yet.
> > There can be a period of time between the value of LPS==1 and the SCLK
> > actually starting.  
> > 
> > fwphy_rddata() appears to be *trying* to deal with this, but is
> > obviously failing.  
> > 
> > So "lps" has been set, but the PHY is not up yet.  In order to access
> > PHY resources, we have to wait for SCLK to start(OHCI spec v1.1 table
> > 6.1).
> > 
> > I believe your error is defined in the OHCI spec, Appendix A.6, PCI Bus
> > Errors.  The bus error is supposed to happen!  :)  The driver just isn't
> > handling the error case properly.
> > 
> > The proper fix is to handle the ERROR according to spec.  I will work on
> > a proper solution this weekend.  In the meantime, here is a patch to get
> > you by based on the pause() mechanism.
> > 


Here is a different, more generic patch that seems to work for me on my
machines.

Essentially, make fwphy_rddata() responsible for catching the error and
implementing the pause().  This *should* have the same effect, unless I
don't understand what I'm doing.

I have eliminated the LPS check for now(see ifdef's) in
fwohci_probe_phy().

If this works, I will cleanup the ifdef's before I commit.

Sean
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fwohci.c.diff
Type: text/x-patch
Size: 5534 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-firewire/attachments/20090420/c4313fb5/fwohci.c.bin