Michal Kazior <michal.kazior@xxxxxxxxx> writes:
> On 26 March 2014 10:28, Kalle Valo <kvalo@xxxxxxxxxxxxxxxx> wrote:
>> Michal Kazior <michal.kazior@xxxxxxxxx> writes:
>
> [...]
>
>>>> - while (wait_limit-- &&
>>>> - !(ioread32(ar_pci->mem + FW_INDICATOR_ADDRESS) &
>>>> - FW_IND_INITIALIZED)) {
>>>> + timeout = jiffies + msecs_to_jiffies(wait);
>>>> +
>>>> + do {
>>>> + val = ath10k_pci_read32(ar, FW_INDICATOR_ADDRESS);
>>>> + if (val == FW_IND_INITIALIZED)
>>>> + break;
>
> Ah, for some reason I've missed this earlier. You seem to replace &
> with ==. I wonder if that's okay?
No, it's not. Good catch, I'll change it back to '&'.
> FW indicator may contain FW_IND_EVENT_PENDING (i.e. firmware crashed
> bit) too.
I'm guessing that FW_IND_INITIALIZED should be the only bit set if
initialisation has happened correctly, but we cannot be certain (right
now). Hence I'll change it back to '&'.
> It might be a good idea to check for that too and return a different
> errno?
Maybe. But to keep this patch simple, I don't add that right now. We can
create a new patch for that.
>>> It might be worth to add:
>>>
>>> if (val == 0xFFFFFFFF)
>>> return -EIO;
>>
>> What does receiving 0xFFFFFFFF mean here? PCI bus kaput?
>
> A simple grepping (find drivers/net/ethernet/ -type f -name '*.[ch]' |
> xargs grep -nC5 ioread32 | grep -C5 -i 0xffffffff) turns out results
> like this:
>
> drivers/net/ethernet/cisco/enic/vnic_rq.c-200- }
> drivers/net/ethernet/cisco/enic/vnic_rq.c-201-
> drivers/net/ethernet/cisco/enic/vnic_rq.c-202- /* Use current
> fetch_index as the ring starting point */
> drivers/net/ethernet/cisco/enic/vnic_rq.c:203: fetch_index =
> ioread32(&rq->ctrl->fetch_index);
> drivers/net/ethernet/cisco/enic/vnic_rq.c-204-
> drivers/net/ethernet/cisco/enic/vnic_rq.c-205- if (fetch_index ==
> 0xFFFFFFFF) { /* check for hardware gone */
> drivers/net/ethernet/cisco/enic/vnic_rq.c-206- /* Hardware
> surprise removal: reset fetch_index */
> drivers/net/ethernet/cisco/enic/vnic_rq.c-207- fetch_index = 0;
> drivers/net/ethernet/cisco/enic/vnic_rq.c-208- }
>
>
>> Do we really want to stop trying after receiving that? What harm would
>> it cause to keep on trying? We would return an error anyway after the
>> timeout, right?
>
> Yes. It's harmless but having the check allows to discern a real
> timeout (target doesn't wake up for some reason) and a pci-e link
> issue.
Ok, I'll add the test you suggested.
--
Kalle Valo
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
