Details
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 48596 Build 45482: arc lint + arc unit
Event Timeline
| sys/dev/ahci/ahci.c | ||
|---|---|---|
| 2688–2689 | whitespace changes | |
| sys/dev/ahci/ahci.h | ||
| 624 | We don't seem to use this quirk? | |
| sys/dev/ahci/ahci_pci.c | ||
| 1 | Unrelated change? /*- is used (in some files) to indicate a preformatted block of text that should not be rewrapped IIRC, most commonly on copyright/license blocks. | |
| 294 | This line just adds a new device that does not need the extended timeouts? | |
This change looks like it still has half of the debugging code in it. That should really be cleaned up. It obscures that the real fix is
| sys/dev/ahci/ahci.c | ||
|---|---|---|
| 2364 | This really isn't timeout, but retry count since you might be reschedule. If you want the time bounded, you should use ab sbtime instead. | |
| 2378–2392 | this code is pointless. We don't ever use last_cr, etc, they are write-only variables. | |
| 2655 | Same comment about the retry count vs elapsed time. Here it likely matters more since we're waiting for a device to connect and rescheduling could lead to rather longer delays which can be more of a problem in the > 10ms raange. though the original code did also suffer from that problem a bit. I also don't like the structure here. Why do we need to increase the timeout only after the first status? that makes no sense, why not have a ?: assignment for timeout_full_limit here? | |
| 2660 | Again, last_status is write-only | |
| 2668 | This especially suggests you'd want to restructure to use a sbtime. | |
| 2671 | We never set this quirk on any device, so let's drop this change until we have one. | |