Patch Set 4: -Code-Review

Patch Set 4:

If you want to keep and fix the driver, I suggest to first
test at what stage (silicon init / notification phase?) FSP
configures things. coreboot's .final seems to run after the
PCI Enum Complete phase, but before the last two phases.

FSP does this in NotifyPhaseApi() - Begin [Phase: 00000020]
Coreboot does it in
Finalize devices...
PCI: 00:17.0 final

This is odd, if FSP runs first and should write a write-once
register, coreboot should not have any effect, right? Maybe
there is a bug in your FSP?

Yes, looks like in my case FSP doesn't even programming those register in notify phase and coreboot is doing wrong 0-2 bit enabling results into SATA device not detected over Port 4. Hence I don't want to much dependent on FSP for such programming at first place and make sure coreboot doing things right if such condition do arise in future.

I don't want to rely on FSP either. But it's there. And Nate once
commented that the point of using Intel's blobs is to have a single
upstream where bug fixes are gathered so every Intel customer can
get them. If we cover FSP bugs up, we defeat its purpose. Don't
worry, I don't mind much. If I'd ever do a port for a new Intel
platform, I would not use FSP-S anyway. It causes too much trouble.

If different FSP binaries do different things with these registers and then we have to do different things in coreboot, maybe we should make this platform-specific.

It might be a bug inside FSP which will fix that not the concern here, but my concern is if FSP misses such that doesn't mean coreboot will perform wrong programming hence the invention was to make common code right to handle PCH-H SATA controller as well. This driver is doing thing right as per register it just because those registers are RW/O hence 2nd time write doesn't take any effect.

@Nico, what should be our direction here, fix this register programming issue or remove those finalize code from here. I'm with first one.

View Change

To view, visit change 41674. To unsubscribe, or for help writing mail filters, visit settings.