fwochi.c and bus_space_barrier()

[Andreas Tobler]

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.
> 
> Sorry for the slow progress here, I appreciate your testing Andreas!

Sean, no need to excuse, I'm busy with a lot of things. My day job, 
well, my day and night job, only permits some hours in the night to play 
around with this cool OS.

For me it is a pleasure to help out testing here. It is a possibility 
for me to learn more about fbsd and its development process.

If I can help more, feed me with code to test. I hope I'll get the 
experience to push out some patches of my own in the future :)

Regards,
Andreas