fwochi.c and bus_space_barrier()

[Andreas Tobler]

Sean Bruno wrote:
> On Wed, 2009-04-15 at 19:37 +0200, Andreas Tobler wrote:
>> Sean Bruno wrote:
>>>>> This looks basically good, but as outlined earlier a driver
>>>>> souldn't busy-wait 50ms. Could one of you please test whether
>>>>> pause("fwlps", (50 * hz + 999) / 1000) works as a drop-in
>>>>> replacement for DELAY(50000) here?
>>>> Works fine here!
>>>>
>>>> Thanks!
>>>> Andreas
>>>>
>>>>
>>>>
>>> Ok, time for more testing.  A couple of changes here.
>>>
>>> 1.  change busy DELAY() call with pause()
>>> 2.  test for lps condition before pause(), if not set pause and retry.
>> Fine here, but see that there was no pause needed, strange.
>>
>> Andreas
>>
>>
> 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