Remove special initialization for ICH8M
ClosedPublic
Actions

Authored by delphij on Nov 22 2018, 7:33 PM.

Details

Reviewers

scottl
mav

Commits

rS349831: MFC r341811 (by delphij):
rS341811: Remove questionable initialization for ICH8M, rely on BIOS to properly

Summary

Remove special initialization for ICH8M, rely on BIOS to properly initialize the controller.

Test Plan

test on Intel based controllers.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 21469
Build 20787: arc lint + arc unit

Event Timeline

delphij created this revision.Nov 22 2018, 7:33 PM

Herald added a subscriber: imp. · View Herald TranscriptNov 22 2018, 7:33 PM

Harbormaster completed remote builds in B21163: Diff 50922.Nov 22 2018, 7:33 PM

I generally believe that we should let BIOS do its job. The chipset specification explicitly tells that OS doesn't need to know about this register. I can't recall why that code exist there and what was the problem, but since it touches only ancient ICH8M chipset, unless there is some other newer cases to have it, I'd remove it completely instead.

I am not sure there is a real reason to move this code after reset, since reset should not affect it, otherwise none of generic AHCI drivers could reset the controller.

Remove ICH8M specific initialization.

Harbormaster completed remote builds in B21469: Diff 51797.Dec 10 2018, 7:20 AM

delphij retitled this revision from Force all ports active after AHCI initialization for Intel controllers. to Remove special initialization for ICH8M.Dec 10 2018, 7:22 AM

delphij edited the summary of this revision. (Show Details)

This changes does what it says, but no clue why we need it.

sys/dev/ahci/ahci_pci.c
361	why?

mav accepted this revision.Dec 10 2018, 6:56 PM

This revision is now accepted and ready to land.Dec 10 2018, 6:56 PM

delphij marked 2 inline comments as done.Dec 10 2018, 7:16 PM

delphij added inline comments.

sys/dev/ahci/ahci_pci.c
361	Because the initialization doesn't seem right. Based on the datasheet, what it does was: If the controller was ICH8M, and if only port 2 (0x04) was enabled among all but port 1 (~0xfe), disable all port and enable port 1. That doesn't really make sense to me, and was not found in anywhere else other than FreeBSD. (Everyone does something similar to ahci_ctrl_reset before poking enabled bits, by the way). The alternative (previous revision) that I have proposed was to enable all ports regardless of their present state, which was done in some other operating systems. However, after talking with mav@, I was convinced that since we are not doing similar thing for so many years now and nobody complained, it's likely that an extra initialization like that was actually redundant.