Change in coreboot[master]: vc/amd/fsp/picasso: add logical to lane number in port descriptor struct
Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43660 )
Change subject: vc/amd/fsp/picasso: add logical to lane number in port descriptor struct ......................................................................
vc/amd/fsp/picasso: add logical to lane number in port descriptor struct
The lane numbers in the PCIe/DXIO descriptor are the logical and not the physical ones, so add logical to the corresponding field names of the fsp_pcie_descriptor struct.
Change-Id: I7037fed225119218e87593932815aff815e83ff8 Signed-off-by: Felix Held felix-coreboot@felixheld.de --- M src/mainboard/amd/mandolin/variants/mandolin/port_descriptors.c M src/mainboard/google/zork/variants/baseboard/fsps_baseboard_dalboz.c M src/mainboard/google/zork/variants/baseboard/fsps_baseboard_trembyle.c M src/vendorcode/amd/fsp/picasso/platform_descriptors.h 4 files changed, 44 insertions(+), 44 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/43660/1
diff --git a/src/mainboard/amd/mandolin/variants/mandolin/port_descriptors.c b/src/mainboard/amd/mandolin/variants/mandolin/port_descriptors.c index 326bcaa..3625554 100644 --- a/src/mainboard/amd/mandolin/variants/mandolin/port_descriptors.c +++ b/src/mainboard/amd/mandolin/variants/mandolin/port_descriptors.c @@ -8,8 +8,8 @@ { /* MXM */ .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = 8, - .end_lane = 15, + .start_logical_lane = 8, + .end_logical_lane = 15, .device_number = 1, .function_number = 1, .link_aspm = ASPM_L1, @@ -21,8 +21,8 @@ { /* SSD */ .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = 0, - .end_lane = 1, + .start_logical_lane = 0, + .end_logical_lane = 1, .device_number = 1, .function_number = 7, .link_aspm = ASPM_L1, @@ -34,8 +34,8 @@ { /* WLAN */ .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = 4, - .end_lane = 4, + .start_logical_lane = 4, + .end_logical_lane = 4, .device_number = 1, .function_number = 2, .link_aspm = ASPM_L1, @@ -47,8 +47,8 @@ { /* LAN */ .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = 5, - .end_lane = 5, + .start_logical_lane = 5, + .end_logical_lane = 5, .device_number = 1, .function_number = 3, .link_aspm = ASPM_L1, @@ -60,8 +60,8 @@ { /* WWAN */ .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = 6, - .end_lane = 6, + .start_logical_lane = 6, + .end_logical_lane = 6, .device_number = 1, .function_number = 4, .link_aspm = ASPM_L1, @@ -73,8 +73,8 @@ { /* WIFI */ .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = 7, - .end_lane = 7, + .start_logical_lane = 7, + .end_logical_lane = 7, .gpio_group_id = 1, .device_number = 1, .function_number = 5, @@ -87,8 +87,8 @@ { /* SATA EXPRESS */ .port_present = true, .engine_type = SATA_ENGINE, - .start_lane = 2, - .end_lane = 3, + .start_logical_lane = 2, + .end_logical_lane = 3, .gpio_group_id = 1, .channel_type = SATA_CHANNEL_LONG, } @@ -98,8 +98,8 @@ { /* MXM */ .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = 8, - .end_lane = 11, + .start_logical_lane = 8, + .end_logical_lane = 11, .device_number = 1, .function_number = 1, .link_aspm = ASPM_L1, @@ -111,8 +111,8 @@ { /* SSD */ .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = 0, - .end_lane = 1, + .start_logical_lane = 0, + .end_logical_lane = 1, .device_number = 1, .function_number = 7, .link_aspm = ASPM_L1, @@ -124,8 +124,8 @@ { /* WLAN */ .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = 4, - .end_lane = 4, + .start_logical_lane = 4, + .end_logical_lane = 4, .device_number = 1, .function_number = 2, .link_aspm = ASPM_L1, @@ -137,8 +137,8 @@ { /* LAN */ .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = 5, - .end_lane = 5, + .start_logical_lane = 5, + .end_logical_lane = 5, .device_number = 1, .function_number = 3, .link_aspm = ASPM_L1, @@ -150,8 +150,8 @@ { /* SATA */ .port_present = true, .engine_type = SATA_ENGINE, - .start_lane = 2, - .end_lane = 3, + .start_logical_lane = 2, + .end_logical_lane = 3, .gpio_group_id = 1, .channel_type = SATA_CHANNEL_LONG, } diff --git a/src/mainboard/google/zork/variants/baseboard/fsps_baseboard_dalboz.c b/src/mainboard/google/zork/variants/baseboard/fsps_baseboard_dalboz.c index b003709..82a11b0 100644 --- a/src/mainboard/google/zork/variants/baseboard/fsps_baseboard_dalboz.c +++ b/src/mainboard/google/zork/variants/baseboard/fsps_baseboard_dalboz.c @@ -18,8 +18,8 @@ // NVME SSD .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = NVME_START_LANE, - .end_lane = NVME_END_LANE, + .start_logical_lane = NVME_START_LANE, + .end_logical_lane = NVME_END_LANE, .device_number = 1, .function_number = 7, .link_aspm = ASPM_L1, @@ -33,8 +33,8 @@ // WLAN .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = WLAN_START_LANE, - .end_lane = WLAN_END_LANE, + .start_logical_lane = WLAN_START_LANE, + .end_logical_lane = WLAN_END_LANE, .device_number = 1, .function_number = 2, .link_aspm = ASPM_L1, @@ -48,8 +48,8 @@ // SD Reader .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = SD_START_LANE, - .end_lane = SD_END_LANE, + .start_logical_lane = SD_START_LANE, + .end_logical_lane = SD_END_LANE, .device_number = 1, .function_number = 3, .link_aspm = ASPM_L1, diff --git a/src/mainboard/google/zork/variants/baseboard/fsps_baseboard_trembyle.c b/src/mainboard/google/zork/variants/baseboard/fsps_baseboard_trembyle.c index df42f6b..9e82684 100644 --- a/src/mainboard/google/zork/variants/baseboard/fsps_baseboard_trembyle.c +++ b/src/mainboard/google/zork/variants/baseboard/fsps_baseboard_trembyle.c @@ -23,8 +23,8 @@ // NVME SSD .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = 0, - .end_lane = 3, + .start_logical_lane = 0, + .end_logical_lane = 3, .device_number = 1, .function_number = 7, .link_aspm = ASPM_L1, @@ -37,8 +37,8 @@ // WLAN .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = 4, - .end_lane = 4, + .start_logical_lane = 4, + .end_logical_lane = 4, .device_number = 1, .function_number = 2, .link_aspm = ASPM_L1, @@ -52,8 +52,8 @@ // SD Reader .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = 5, - .end_lane = 5, + .start_logical_lane = 5, + .end_logical_lane = 5, .device_number = 1, .function_number = 3, .link_aspm = ASPM_L1, @@ -69,8 +69,8 @@ // NVME SSD .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = NVME_START_LANE, - .end_lane = NVME_END_LANE, + .start_logical_lane = NVME_START_LANE, + .end_logical_lane = NVME_END_LANE, .device_number = 1, .function_number = 7, .link_aspm = ASPM_L1, @@ -84,8 +84,8 @@ // WLAN .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = WLAN_START_LANE, - .end_lane = WLAN_END_LANE, + .start_logical_lane = WLAN_START_LANE, + .end_logical_lane = WLAN_END_LANE, .device_number = 1, .function_number = 2, .link_aspm = ASPM_L1, @@ -99,8 +99,8 @@ // SD Reader .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = SD_START_LANE, - .end_lane = SD_END_LANE, + .start_logical_lane = SD_START_LANE, + .end_logical_lane = SD_END_LANE, .device_number = 1, .function_number = 3, .link_aspm = ASPM_L1, diff --git a/src/vendorcode/amd/fsp/picasso/platform_descriptors.h b/src/vendorcode/amd/fsp/picasso/platform_descriptors.h index 9765ea6..acf821b 100644 --- a/src/vendorcode/amd/fsp/picasso/platform_descriptors.h +++ b/src/vendorcode/amd/fsp/picasso/platform_descriptors.h @@ -113,8 +113,8 @@ /* Beware that the lane numbers in here are the logical and not the physical lane numbers! */ typedef struct __packed { uint8_t engine_type; - uint8_t start_lane; // Start lane of the pci device - uint8_t end_lane; // End lane of the pci device + uint8_t start_logical_lane; // Start lane of the pci device + uint8_t end_logical_lane; // End lane of the pci device uint8_t gpio_group_id; // FCH reset number. 0 is global reset uint32_t port_present :1; // Should be TRUE if train link uint32_t reserved_3 :7;
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43660 )
Change subject: vc/amd/fsp/picasso: add logical to lane number in port descriptor struct ......................................................................
Patch Set 2: Code-Review+2
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43660 )
Change subject: vc/amd/fsp/picasso: add logical to lane number in port descriptor struct ......................................................................
vc/amd/fsp/picasso: add logical to lane number in port descriptor struct
The lane numbers in the PCIe/DXIO descriptor are the logical and not the physical ones, so add logical to the corresponding field names of the fsp_pcie_descriptor struct.
Change-Id: I7037fed225119218e87593932815aff815e83ff8 Signed-off-by: Felix Held felix-coreboot@felixheld.de Reviewed-on: https://review.coreboot.org/c/coreboot/+/43660 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Aaron Durbin adurbin@chromium.org --- M src/mainboard/amd/mandolin/variants/mandolin/port_descriptors.c M src/mainboard/google/zork/variants/baseboard/fsps_baseboard_dalboz.c M src/mainboard/google/zork/variants/baseboard/fsps_baseboard_trembyle.c M src/vendorcode/amd/fsp/picasso/platform_descriptors.h 4 files changed, 44 insertions(+), 44 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved
diff --git a/src/mainboard/amd/mandolin/variants/mandolin/port_descriptors.c b/src/mainboard/amd/mandolin/variants/mandolin/port_descriptors.c index 326bcaa..3625554 100644 --- a/src/mainboard/amd/mandolin/variants/mandolin/port_descriptors.c +++ b/src/mainboard/amd/mandolin/variants/mandolin/port_descriptors.c @@ -8,8 +8,8 @@ { /* MXM */ .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = 8, - .end_lane = 15, + .start_logical_lane = 8, + .end_logical_lane = 15, .device_number = 1, .function_number = 1, .link_aspm = ASPM_L1, @@ -21,8 +21,8 @@ { /* SSD */ .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = 0, - .end_lane = 1, + .start_logical_lane = 0, + .end_logical_lane = 1, .device_number = 1, .function_number = 7, .link_aspm = ASPM_L1, @@ -34,8 +34,8 @@ { /* WLAN */ .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = 4, - .end_lane = 4, + .start_logical_lane = 4, + .end_logical_lane = 4, .device_number = 1, .function_number = 2, .link_aspm = ASPM_L1, @@ -47,8 +47,8 @@ { /* LAN */ .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = 5, - .end_lane = 5, + .start_logical_lane = 5, + .end_logical_lane = 5, .device_number = 1, .function_number = 3, .link_aspm = ASPM_L1, @@ -60,8 +60,8 @@ { /* WWAN */ .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = 6, - .end_lane = 6, + .start_logical_lane = 6, + .end_logical_lane = 6, .device_number = 1, .function_number = 4, .link_aspm = ASPM_L1, @@ -73,8 +73,8 @@ { /* WIFI */ .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = 7, - .end_lane = 7, + .start_logical_lane = 7, + .end_logical_lane = 7, .gpio_group_id = 1, .device_number = 1, .function_number = 5, @@ -87,8 +87,8 @@ { /* SATA EXPRESS */ .port_present = true, .engine_type = SATA_ENGINE, - .start_lane = 2, - .end_lane = 3, + .start_logical_lane = 2, + .end_logical_lane = 3, .gpio_group_id = 1, .channel_type = SATA_CHANNEL_LONG, } @@ -98,8 +98,8 @@ { /* MXM */ .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = 8, - .end_lane = 11, + .start_logical_lane = 8, + .end_logical_lane = 11, .device_number = 1, .function_number = 1, .link_aspm = ASPM_L1, @@ -111,8 +111,8 @@ { /* SSD */ .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = 0, - .end_lane = 1, + .start_logical_lane = 0, + .end_logical_lane = 1, .device_number = 1, .function_number = 7, .link_aspm = ASPM_L1, @@ -124,8 +124,8 @@ { /* WLAN */ .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = 4, - .end_lane = 4, + .start_logical_lane = 4, + .end_logical_lane = 4, .device_number = 1, .function_number = 2, .link_aspm = ASPM_L1, @@ -137,8 +137,8 @@ { /* LAN */ .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = 5, - .end_lane = 5, + .start_logical_lane = 5, + .end_logical_lane = 5, .device_number = 1, .function_number = 3, .link_aspm = ASPM_L1, @@ -150,8 +150,8 @@ { /* SATA */ .port_present = true, .engine_type = SATA_ENGINE, - .start_lane = 2, - .end_lane = 3, + .start_logical_lane = 2, + .end_logical_lane = 3, .gpio_group_id = 1, .channel_type = SATA_CHANNEL_LONG, } diff --git a/src/mainboard/google/zork/variants/baseboard/fsps_baseboard_dalboz.c b/src/mainboard/google/zork/variants/baseboard/fsps_baseboard_dalboz.c index b003709..82a11b0 100644 --- a/src/mainboard/google/zork/variants/baseboard/fsps_baseboard_dalboz.c +++ b/src/mainboard/google/zork/variants/baseboard/fsps_baseboard_dalboz.c @@ -18,8 +18,8 @@ // NVME SSD .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = NVME_START_LANE, - .end_lane = NVME_END_LANE, + .start_logical_lane = NVME_START_LANE, + .end_logical_lane = NVME_END_LANE, .device_number = 1, .function_number = 7, .link_aspm = ASPM_L1, @@ -33,8 +33,8 @@ // WLAN .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = WLAN_START_LANE, - .end_lane = WLAN_END_LANE, + .start_logical_lane = WLAN_START_LANE, + .end_logical_lane = WLAN_END_LANE, .device_number = 1, .function_number = 2, .link_aspm = ASPM_L1, @@ -48,8 +48,8 @@ // SD Reader .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = SD_START_LANE, - .end_lane = SD_END_LANE, + .start_logical_lane = SD_START_LANE, + .end_logical_lane = SD_END_LANE, .device_number = 1, .function_number = 3, .link_aspm = ASPM_L1, diff --git a/src/mainboard/google/zork/variants/baseboard/fsps_baseboard_trembyle.c b/src/mainboard/google/zork/variants/baseboard/fsps_baseboard_trembyle.c index df42f6b..9e82684 100644 --- a/src/mainboard/google/zork/variants/baseboard/fsps_baseboard_trembyle.c +++ b/src/mainboard/google/zork/variants/baseboard/fsps_baseboard_trembyle.c @@ -23,8 +23,8 @@ // NVME SSD .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = 0, - .end_lane = 3, + .start_logical_lane = 0, + .end_logical_lane = 3, .device_number = 1, .function_number = 7, .link_aspm = ASPM_L1, @@ -37,8 +37,8 @@ // WLAN .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = 4, - .end_lane = 4, + .start_logical_lane = 4, + .end_logical_lane = 4, .device_number = 1, .function_number = 2, .link_aspm = ASPM_L1, @@ -52,8 +52,8 @@ // SD Reader .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = 5, - .end_lane = 5, + .start_logical_lane = 5, + .end_logical_lane = 5, .device_number = 1, .function_number = 3, .link_aspm = ASPM_L1, @@ -69,8 +69,8 @@ // NVME SSD .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = NVME_START_LANE, - .end_lane = NVME_END_LANE, + .start_logical_lane = NVME_START_LANE, + .end_logical_lane = NVME_END_LANE, .device_number = 1, .function_number = 7, .link_aspm = ASPM_L1, @@ -84,8 +84,8 @@ // WLAN .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = WLAN_START_LANE, - .end_lane = WLAN_END_LANE, + .start_logical_lane = WLAN_START_LANE, + .end_logical_lane = WLAN_END_LANE, .device_number = 1, .function_number = 2, .link_aspm = ASPM_L1, @@ -99,8 +99,8 @@ // SD Reader .port_present = true, .engine_type = PCIE_ENGINE, - .start_lane = SD_START_LANE, - .end_lane = SD_END_LANE, + .start_logical_lane = SD_START_LANE, + .end_logical_lane = SD_END_LANE, .device_number = 1, .function_number = 3, .link_aspm = ASPM_L1, diff --git a/src/vendorcode/amd/fsp/picasso/platform_descriptors.h b/src/vendorcode/amd/fsp/picasso/platform_descriptors.h index 9765ea6..acf821b 100644 --- a/src/vendorcode/amd/fsp/picasso/platform_descriptors.h +++ b/src/vendorcode/amd/fsp/picasso/platform_descriptors.h @@ -113,8 +113,8 @@ /* Beware that the lane numbers in here are the logical and not the physical lane numbers! */ typedef struct __packed { uint8_t engine_type; - uint8_t start_lane; // Start lane of the pci device - uint8_t end_lane; // End lane of the pci device + uint8_t start_logical_lane; // Start lane of the pci device + uint8_t end_logical_lane; // End lane of the pci device uint8_t gpio_group_id; // FCH reset number. 0 is global reset uint32_t port_present :1; // Should be TRUE if train link uint32_t reserved_3 :7;
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43660 )
Change subject: vc/amd/fsp/picasso: add logical to lane number in port descriptor struct ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43660/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43660/4//COMMIT_MSG@11 PS4, Line 11: fsp_pcie_descriptor struct. Felix, where is the physical to logical mapping determined? I can't piece it together with the docs I have.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43660 )
Change subject: vc/amd/fsp/picasso: add logical to lane number in port descriptor struct ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43660/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43660/4//COMMIT_MSG@11 PS4, Line 11: fsp_pcie_descriptor struct.
Felix, where is the physical to logical mapping determined? I can't piece it together with the docs […]
I think I found it in the PPR, but I don't see controller mappings unless they are implicit.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43660 )
Change subject: vc/amd/fsp/picasso: add logical to lane number in port descriptor struct ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43660/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43660/4//COMMIT_MSG@11 PS4, Line 11: fsp_pcie_descriptor struct.
I think I found it in the PPR, but I don't see controller mappings unless they are implicit.
for Picasso and Dali there is some documentation in the PPR on that; the mapping on Pollock is different to that. I'll see that I can get that documented
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43660 )
Change subject: vc/amd/fsp/picasso: add logical to lane number in port descriptor struct ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43660/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43660/4//COMMIT_MSG@11 PS4, Line 11: fsp_pcie_descriptor struct.
for Picasso and Dali there is some documentation in the PPR on that; the mapping on Pollock is diffe […]
https://b.corp.google.com/issues/162423378 has all the many pieces of confusion and lack of clarity in the documentation. Those phy sharing tables don't appear correct for the actual combinations even for dali/picasso. And, as you noted, Pollock has *no documentation* that I can access.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43660 )
Change subject: vc/amd/fsp/picasso: add logical to lane number in port descriptor struct ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43660/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43660/4//COMMIT_MSG@11 PS4, Line 11: fsp_pcie_descriptor struct.
The documentation in that ticket matches what I'd expect for Picasso/Dali; have to verify the mapping of the m.2 lanes that partially support both SATA and PCIe. Were the zork devices also tested with a SATA m.2 SSD or only with PCIe ones?
For Pollock the logical lane numbers match with what I'd expect, since it's an RV2 die; the mapping to the physical/GPP lane numbers is however different than I had expected. I probably don't have that much more information on Pollock than you have access to.
-
Aaron Durbin (Code Review) -
Felix Held (Code Review)