Andrey Petrov has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39918 )
Change subject: soc/intel/xeon_sp/cpx: Enable SATA ports ......................................................................
soc/intel/xeon_sp/cpx: Enable SATA ports
Looks like FSP does not explicity configure SATA ports as enabled. As result some payloads (SeaBIOS, TianoCore) can detect the drives but Linux kernel does not. Turns out the kernel does not touch disabled ports, while SeaBIOS just checks all available ports. Interestingly, SKX FSP seems to be enabling all the ports.
This change hooks up some common SATA code which enables all ports.
TEST=booted on Cedar Island CRB, make sure 7-pin SATA drive works
Change-Id: Iba8f8c8812168deace1abaa7cf3996b870648686 --- M src/soc/intel/xeon_sp/cpx/Kconfig 1 file changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/39918/1
diff --git a/src/soc/intel/xeon_sp/cpx/Kconfig b/src/soc/intel/xeon_sp/cpx/Kconfig index 70703d0..2f637b1 100644 --- a/src/soc/intel/xeon_sp/cpx/Kconfig +++ b/src/soc/intel/xeon_sp/cpx/Kconfig @@ -17,6 +17,8 @@ select POSTCAR_CONSOLE select POSTCAR_STAGE select FSP_USES_CB_STACK + select SOC_INTEL_COMMON_BLOCK_SATA + select SOC_AHCI_PORT_IMPLEMENTED_INVERT
config FSP_HEADER_PATH string "Location of FSP headers"
Hello Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39918
to look at the new patch set (#2).
Change subject: soc/intel/xeon_sp/cpx: Enable SATA ports ......................................................................
soc/intel/xeon_sp/cpx: Enable SATA ports
Looks like FSP does not explicity configure SATA ports as enabled. As result some payloads (SeaBIOS, TianoCore) can detect the drives but Linux kernel does not. Turns out the kernel does not touch disabled ports, while SeaBIOS just checks all available ports. Interestingly, SKX FSP seems to be enabling all the ports.
This change hooks up some common SATA code which enables all ports.
TEST=booted on Cedar Island CRB, make sure 7-pin SATA drive works
Change-Id: Iba8f8c8812168deace1abaa7cf3996b870648686 Signed-off-by: Andrey Petrov anpetrov@fb.com --- M src/soc/intel/xeon_sp/cpx/Kconfig 1 file changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/39918/2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39918 )
Change subject: soc/intel/xeon_sp/cpx: Enable SATA ports ......................................................................
Patch Set 2: Code-Review+1
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39918 )
Change subject: soc/intel/xeon_sp/cpx: Enable SATA ports ......................................................................
Patch Set 2: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/39918/2/src/soc/intel/xeon_sp/cpx/K... File src/soc/intel/xeon_sp/cpx/Kconfig:
https://review.coreboot.org/c/coreboot/+/39918/2/src/soc/intel/xeon_sp/cpx/K... PS2, Line 20: SOC_INTEL_COMMON_BLOCK_SATA It seems to me that the mask in the common code https://github.com/coreboot/coreboot/blob/7eeaeeecc590d22a8f51175ba07cf2cfad... isn't correct, because in the C620 we have 8 ports, as in the 200/300 series chipset. In this case, we can only change the state of the first four ports. What do you think about this? Maybe this mask is valid for mobile versions of processors.
Andrey Petrov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39918 )
Change subject: soc/intel/xeon_sp/cpx: Enable SATA ports ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39918/2/src/soc/intel/xeon_sp/cpx/K... File src/soc/intel/xeon_sp/cpx/Kconfig:
https://review.coreboot.org/c/coreboot/+/39918/2/src/soc/intel/xeon_sp/cpx/K... PS2, Line 20: SOC_INTEL_COMMON_BLOCK_SATA
It seems to me that the mask in the common code https://github. […]
good catch. I guess we will have add that logic (number of ports) directly in block/sata/sata.c, since it looks like it is not possible to figure out hardware capabilities run-time. I will work on a patch for this
David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39918 )
Change subject: soc/intel/xeon_sp/cpx: Enable SATA ports ......................................................................
Patch Set 2: Code-Review+1
This also works for Skylake-SP, so please add the appropriate Kconfig options there as well.
Attention is currently required from: Andrey Petrov. Rocky Phagura has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39918 )
Change subject: soc/intel/xeon_sp/cpx: Enable SATA ports ......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2: Hi, I'm doing a cleanup of xeon_sp patches. This is an old patch. Is this still being worked on? If not, we can move it to WIP, private, or abandon state. Currently the SOC_INTEL_COMMON_BLOCK_SATA is being selected in the PCH Kconfig so this patch may not be needed anymore.
Stefan Reinauer has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/39918?usp=email )
Change subject: soc/intel/xeon_sp/cpx: Enable SATA ports ......................................................................
Abandoned
-
Andrey Petrov (Code Review) -
David Hendricks (Code Review)
-
Maxim Polyakov (Code Review)
-
Paul Menzel (Code Review)
-
Rocky Phagura (Code Review)
-
Stefan Reinauer (Code Review)