Re: CVS commit: src/sys/dev

[Matthias Drochner <M.Drochner%fz-juelich.de@localhost>]

dyoung%NetBSD.org@localhost said:
> Disable pci_disable_retry() and cardbus_disable_retry(), with a long
> comment on pci_disable_retry() (cross-referenced at
> cardbus_disable_retry()) that explains why. 

I fully agree that this stuff should not be in generic PCI
code. Config space >=0x40 is device specific (except the
bits references through the capability list).
Without documentation for the chips affected I can only guess,
but one interpretation tells that setting this register might
be necessary for correct operation, at least in the future.

Disabling retries is nonsense of course. As you say a PCI
master is required to retry eternally. So what is likely
disabled is that there is an arbitrary limit to the retries.
So the config register write would restore correct behaviour.
Your observation on ath doesn't agree, but since the whole
thing is non-standard this doesn't tell about other chips.
(If the ath chip does give up w/o retry, there should be
at least some error bit set somewhere...)

The retry timeout might be long enough to never fire
in normal operation. The original comment mentions
interactions with CPU C3 state. In this state, the CPU
cache doesn't snoop. I didn't check chipset documentation
yet, but a reasonable behaviour would be to just have
all external bus masters retry their transactions.
If the CPU stays in C3 for longer than the retry timeout,
data loss would occur.

Afaict NetBSD doesn't put the CPU into anything lower
than C1 unless suspending (it would need some scheduler
support to weight latencies). So there might be no
problem yet. Other OSes do, and NetBSD might add it
eventually.

Anyway, the code should be put back into the affected drivers,
and removed from generic PCI code for sanity.

best regards
Matthias




-------------------------------------------------------------------
-------------------------------------------------------------------
Forschungszentrum Juelich GmbH
52425 Juelich

Sitz der Gesellschaft: Juelich
Eingetragen im Handelsregister des Amtsgerichts Dueren Nr. HR B 3498
Vorsitzende des Aufsichtsrats: MinDir'in Baerbel Brumme-Bothe
Geschaeftsfuehrung: Prof. Dr. Achim Bachem (Vorsitzender),
Dr. Ulrich Krafft (stellv. Vorsitzender), Dr. Sebastian M. Schmidt
-------------------------------------------------------------------
-------------------------------------------------------------------