Introduce Annapurna Labs AHCI support

[zbb]

Summary

Overview:    
* implemented quirk for forcing SATA interface enable
* restore value to status register - this enables link autonegotiation

Modifications:
* devid:vendorid field
* quirk for forcing PI setting (BIOS is doing that on PC-like systems)
* write to capabilites field to enable phy link initialization

Submitted by: Wojciech Macek <[e-mail]>
Obtained from: Semihalf

[imp]

sys/dev/ahci/ahci.c

150–153 ↗

(On Diff #4130)

Playing devil's advocate: why not do this always? Or, why do we need to do it at all? Is this an erratum for the Annapurna parts, or is how the Annapurna parts being used different so we only see it there?

199–201 ↗

(On Diff #4130)

Why don't other cards need this? What makes Annapurna cards special? Is this something the spec says we should be doing, but aren't? Is there some other way to know than a big giant table this is required?

[zbb]

sys/dev/ahci/ahci.c

150–153 ↗

(On Diff #4130)

Jakub Palider wrote:

Yes, actually this is how things are expected to look like in general, I
mean restore caps. But we have no way to check it on other platforms at
the moment. If you think this does not mess up other configurations it
would be fine to perform the operation on a normal basis.

199–201 ↗

(On Diff #4130)

Jakub Palider wrote:

This is something bootloaders may set up or not. This way we run
independently and make SATAs operational. This also allows to fiddle
around if someone feels like experimenting...

[mav]

sys/dev/ahci/ahci.c

150–153 ↗

(On Diff #4130)

AHCI specification defines this register as read-only. It tells nothing about "restoring" this register. I don't like idea of writing to read-only registers.

199–201 ↗

(On Diff #4130)

According to specification, this is something that platform/BIOS must set. I know that Linux sets it always, but I believe that this is spec violation. If there is some BROKEN hardware that require it to be set by the driver, I can accept hardware-specific quirk or some tunable, but not global.

[zbb]

sys/dev/ahci/ahci.c

150–153 ↗

(On Diff #4130)

Jakub Palider wrote:

I have overlooked it in the specs and took the CAP register RW for
granted. You are right, writing to RO registers doesn't sound fine, and
we don't like it either. But we have little choice but to perform the
writes to make it operational.
You say adding it to quirks is not proper place? Where then?

199–201 ↗

(On Diff #4130)

Jakub Palider wrote:

I'd prefer read MAY/SHOULD/MUST but the spec simply says that it is
loaded by the BIOS. However in reality that does not always happen and
higher level stuff suffers from broken implementations. Linux does it
and although it is not our reference it shows a way to just make things
work. Again - is adding platform quirks wrong way?

[mav]

sys/dev/ahci/ahci.c

199–201 ↗

(On Diff #4130)

I haven't told that something is wrong with the quirk. It is fine to me, if it is really needed, that is I am not sure about.

[zbb]

So what do we do with this patch?

[imp]

sys/dev/ahci/ahci.c

150–153 ↗

(On Diff #4130)

We should document this then. Add a comment that states that some platforms, specifically BLAH, require writing to this read-only register so a quirk is provided to update them and prevent writing to standard conforming devices. This way we know we can eliminate it if it turns out that the current root-cause analysis proves to be inaccurate or better worked around.

199–201 ↗

(On Diff #4130)

I'd add a comment here as well so we know why we do this weird, odd-ball thing. It will also help reduce the temptation to cut and paste it for devices that clearly don't need it.

[imp]

Thanks for the comments. Still not sure what an Annapurna is....

[zbb]

Thank you very much. Committed to:
https://svnweb.freebsd.org/changeset/base/280184