fwochi.c and bus_space_barrier()

[Sean Bruno]

> >>
> > 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.

Sorry for the slow progress here, I appreciate your testing Andreas!

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