Hello Patrick Rudolph, Mimoja, Felix Singer, build bot (Jenkins), Nico Huber, Patrick Georgi,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/37094
to review the following change.
Change subject: Revert "3rdparts/fsp: Update fsp submodule" ......................................................................
Revert "3rdparts/fsp: Update fsp submodule"
This reverts commit 0b82b3d6fdc413db5b65b08adc0d2a5ddc6687ad.
Reason for revert: Cannonlake FSP does not work anymore. FSP-M crashes/get stuck.
Change-Id: I58ee2d7ee2c12558498f02219ee45d61d942077e Signed-off-by: Christian Walter christian.walter@9elements.com --- M 3rdparty/fsp M src/soc/intel/cannonlake/Kconfig 2 files changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/37094/1
diff --git a/3rdparty/fsp b/3rdparty/fsp index 0bc2b07..5996417 160000 --- a/3rdparty/fsp +++ b/3rdparty/fsp @@ -1 +1 @@ -Subproject commit 0bc2b07eab29a8a75cd084963c285ee5434e6666 +Subproject commit 59964173e18950debcc6b8856c5c928935ce0b4f diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig index c82660e..5c91ac1 100644 --- a/src/soc/intel/cannonlake/Kconfig +++ b/src/soc/intel/cannonlake/Kconfig @@ -310,7 +310,7 @@ config FSP_FD_PATH string depends on FSP_USE_REPO - default "3rdparty/fsp/CoffeeLakeFspBinPkg/Fsp.fd" if SOC_INTEL_COFFEELAKE || SOC_INTEL_WHISKEYLAKE + default "3rdparty/fsp/CoffeeLakeFspBinPkg/FSP.fd" if SOC_INTEL_COFFEELAKE || SOC_INTEL_WHISKEYLAKE
config SOC_INTEL_CANNONLAKE_DEBUG_CONSENT int "Debug Consent for CNL"
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37094 )
Change subject: Revert "3rdparts/fsp: Update fsp submodule" ......................................................................
Patch Set 1: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37094 )
Change subject: Revert "3rdparts/fsp: Update fsp submodule" ......................................................................
Patch Set 1: Code-Review+1
Please paste the log in the commit message.
Hello Patrick Rudolph, Mimoja, Paul Menzel, Felix Singer, build bot (Jenkins), Nico Huber, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37094
to look at the new patch set (#2).
Change subject: Revert "3rdparts/fsp: Update fsp submodule" ......................................................................
Revert "3rdparts/fsp: Update fsp submodule"
This reverts commit 0b82b3d6fdc413db5b65b08adc0d2a5ddc6687ad.
Reason for revert: Cannonlake FSP does not work anymore. FSP-M crashes/get stuck.
Log in romstage:
FMAP: area COREBOOT found @ 1610200 (10419712 bytes) CBFS: 'COREBOOT Locator' located CBFS at [1610200:2000000) CBFS: Locating 'fspm.bin' CBFS: Found @ offset 84dc0 size 88000 POST: 0x34 FMAP: area RW_MRC_CACHE found @ 1600000 (65536 bytes) PRMRR disabled by config. POST: 0x36 POST: 0x92
no response after FSP-M call.
Change-Id: I58ee2d7ee2c12558498f02219ee45d61d942077e Signed-off-by: Christian Walter christian.walter@9elements.com --- M 3rdparty/fsp M src/soc/intel/cannonlake/Kconfig 2 files changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/37094/2
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37094 )
Change subject: Revert "3rdparts/fsp: Update fsp submodule" ......................................................................
Patch Set 2:
Patch Set 1: Code-Review+1
Please paste the log in the commit message.
ACK.
Mimoja has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37094 )
Change subject: Revert "3rdparts/fsp: Update fsp submodule" ......................................................................
Patch Set 2:
How are we planing on dealing with this longterm? Reverting this would let fall Ice Lake out of support again... Could it be a temporary solution to keep the last known-good FSP.fd in the blobs repo for now and raise the problem with Intel or figure out the problem?
It might also be a quick solution to test the commit 9e53d779eb34e944f9b3386ad6a9df80f710bddd, that updated most but is before the f3ecfc496e9c07b553582197b5086cfe9948b384 commit "Coffee Lake FSP 7.0.68.40" and "Coffee Lake FSP 7.0.68.41" later that same day, so it still contains the CoffeLake FSP 7.0.64.40 Version from May 23rd.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37094 )
Change subject: Revert "3rdparts/fsp: Update fsp submodule" ......................................................................
Patch Set 2: Code-Review-1
(1 comment)
https://review.coreboot.org/c/coreboot/+/37094/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37094/2//COMMIT_MSG@11 PS2, Line 11: Reason for revert: Cannonlake FSP does not work anymore. FSP-M crashes/get stuck. IMHO, "does not work" is the most generic description of an issue. Currently, there is not enough information to reproduce this issue.
- Which mainboard does this happen with? With what coreboot code, specifically? - Which defconfig has been used that results in this boot failure? - Has anything else been tried before reverting?
On github IntelFSP/FSP, the difference between
59964173e18950debcc6b8856c5c928935ce0b4f
and
0bc2b07eab29a8a75cd084963c285ee5434e6666
is of 56 commits, which gets bisected in at most *six* tests. I am pretty sure that even less tests are required, because not every commit changes the Cannonlake FSP (did you mean CoffeeLake?).
Mimoja has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37094 )
Change subject: Revert "3rdparts/fsp: Update fsp submodule" ......................................................................
Patch Set 2:
Patch Set 2:
How are we planing on dealing with this longterm? Reverting this would let fall Ice Lake out of support again... Could it be a temporary solution to keep the last known-good FSP.fd in the blobs repo for now and raise the problem with Intel or figure out the problem?
It might also be a quick solution to test the commit 9e53d779eb34e944f9b3386ad6a9df80f710bddd, that updated most but is before the f3ecfc496e9c07b553582197b5086cfe9948b384 commit "Coffee Lake FSP 7.0.68.40" and "Coffee Lake FSP 7.0.68.41" later that same day, so it still contains the CoffeLake FSP 7.0.64.40 Version from May 23rd.
I think i made a mistake.. Cannonlake is derived from Comet and not from CoffeeLake, right?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37094 )
Change subject: Revert "3rdparts/fsp: Update fsp submodule" ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37094/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37094/2//COMMIT_MSG@11 PS2, Line 11: Reason for revert: Cannonlake FSP does not work anymore. FSP-M crashes/get stuck.
IMHO, "does not work" is the most generic description of an issue. […]
Look, there's less than six individual commits that might be of interest:
[1] https://github.com/IntelFsp/FSP/commit/cd201fc17c7e315d2ab4cb40d53427893d9d7... [2] https://github.com/IntelFsp/FSP/commit/573bd5d6861376c8b4947d177dfe70592ff80... [3] https://github.com/IntelFsp/FSP/commit/5391d1fd1c3eeddef588f7a9bdeeee562761d... [4] https://github.com/IntelFsp/FSP/commit/f3ecfc496e9c07b553582197b5086cfe9948b... [5] https://github.com/IntelFsp/FSP/commit/3421e5ead1f336c7b3024dd8e42a9c05749e3...
Please test which specific commit breaks FSP.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37094 )
Change subject: Revert "3rdparts/fsp: Update fsp submodule" ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37094/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37094/2//COMMIT_MSG@11 PS2, Line 11: Reason for revert: Cannonlake FSP does not work anymore. FSP-M crashes/get stuck.
Look, there's less than six individual commits that might be of interest: […]
* It's a blob, what else should be done? * It works when reverted thus Kconfig is not relevant * No Feel free to bisect.
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37094 )
Change subject: Revert "3rdparts/fsp: Update fsp submodule" ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37094/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37094/2//COMMIT_MSG@11 PS2, Line 11: Reason for revert: Cannonlake FSP does not work anymore. FSP-M crashes/get stuck.
- It's a blob, what else should be done? […]
folks, could you re-try this part of the review and do it in a less adversarial way, please?
I think the request to figure out better what's going on is fine. Also, is there communication with Intel going on (e.g. on Github) to sort out what's going on there?
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37094 )
Change subject: Revert "3rdparts/fsp: Update fsp submodule" ......................................................................
Patch Set 2:
Subrata, could you please find somebody who's responsible for that FSP who can look into it ASAP?
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37094 )
Change subject: Revert "3rdparts/fsp: Update fsp submodule" ......................................................................
Patch Set 2:
Okay lets ease down here. I just double checked. What we could to is change this here - and instead of reverting it, just use commit 573bd5d6861376c8b4947d177dfe70592ff80fc2 (which is the renaming one) and use this as the submodule commit for now.
To be honest - I dont think it's my responsibility to debug the FSP now (how could I) - I am just notifying about the error - reverting this and we are go to go again.
Commit 573bd5d6861376c8b4947d177dfe70592ff80fc2 seems to work.
Mimoja has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37094 )
Change subject: Revert "3rdparts/fsp: Update fsp submodule" ......................................................................
Patch Set 2:
Patch Set 2:
Okay lets ease down here. I just double checked. What we could to is change this here - and instead of reverting it, just use commit 573bd5d6861376c8b4947d177dfe70592ff80fc2 (which is the renaming one) and use this as the submodule commit for now.
To be honest - I dont think it's my responsibility to debug the FSP now (how could I) - I am just notifying about the error - reverting this and we are go to go again.
Commit 573bd5d6861376c8b4947d177dfe70592ff80fc2 seems to work.
Thanks for checking. I am surprised that 5391d1f is already bad, but okay. Can we use 5391d1f^ as new submodule commit?
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37094 )
Change subject: Revert "3rdparts/fsp: Update fsp submodule" ......................................................................
Patch Set 2:
Patch Set 2: Okay lets ease down here. I just double checked. What we could to is change this here - and instead of reverting it, just use commit 573bd5d6861376c8b4947d177dfe70592ff80fc2 (which is the renaming one) and use this as the submodule commit for now.
That would keep the rename part of the commit intact, but downgrade a bunch of FSPs, right?
To be honest - I dont think it's my responsibility to debug the FSP now (how could I) - I am just notifying about the error - reverting this and we are go to go again.
It's not your responsibility to debug, right. Doing a proper deep dive into the issue is on Intel (given that they own FSP and do all these secrecy games), but nobody asked you to do that.
It's common courtesy around here to support folks when you see an issue (that may or may not be reproducible for others) by limiting the search space though, and that's what Angel asked for when requesting information that helps reproduce the issue. Since you report the issue, you're the only one that we know has the hardware to do that, hence the request to you. (there's room for improvement on the presentation of that request though, as is with the responses)
A result of such information might be (depending on when the issue was introduced exactly) that we can revert to a commit that keeps other FSPs working while still resetting Cannonlake to a working earlier state.
Right now you're asking to break other chipsets (such as Ice Lake) to fix yours, and people can play that game for a long time with everybody feeling justified about their actions.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37094 )
Change subject: Revert "3rdparts/fsp: Update fsp submodule" ......................................................................
Patch Set 2:
Patch Set 2:
Patch Set 2:
How are we planing on dealing with this longterm? Reverting this would let fall Ice Lake out of support again... Could it be a temporary solution to keep the last known-good FSP.fd in the blobs repo for now and raise the problem with Intel or figure out the problem?
It might also be a quick solution to test the commit 9e53d779eb34e944f9b3386ad6a9df80f710bddd, that updated most but is before the f3ecfc496e9c07b553582197b5086cfe9948b384 commit "Coffee Lake FSP 7.0.68.40" and "Coffee Lake FSP 7.0.68.41" later that same day, so it still contains the CoffeLake FSP 7.0.64.40 Version from May 23rd.
I think i made a mistake.. Cannonlake is derived from Comet and not from CoffeeLake, right?
Cannonlake was meant to be the successor to Kaby Lake, with a shrink from 14nm to 10nm. However, AFAIUI, that shrink caused yield issues, and the only commercial Cannonlake chip is the i3-8121U. Instead, Coffee Lake was released, which uses a 14nm process.
I found this table particularly useful: https://en.wikipedia.org/wiki/Tick%E2%80%93tock_model#Roadmap
Patch Set 2:
Okay lets ease down here. I just double checked. What we could to is change this here - and instead of reverting it, just use commit 573bd5d6861376c8b4947d177dfe70592ff80fc2 (which is the renaming one) and use this as the submodule commit for now.
To be honest - I dont think it's my responsibility to debug the FSP now (how could I) - I am just notifying about the error - reverting this and we are go to go again.
Commit 573bd5d6861376c8b4947d177dfe70592ff80fc2 seems to work.
Alright, thanks for testing. This would narrow the issue down to three commits, one of which is just a documentation change. Therefore, the other two commits are extremely likely to be where the regression was introduced.
These last two commits correspond to the FSP 7.0.68.40 and FSP 7.0.68.41 updates for CFL. Do note that the former adds two additional FSP UPDs:
- LctRelaxedReset - ScsSdCardWpPinEnabled
The latter is a FSP-S UPD related to the SD card controller's Write-Protect pin. I don't think it's much of an issue.
However, the other UPD is a FSP-M UPD, and is specific to DDR4 memory! The UPD description says:
Late Command Training Relaxed Reset Enable/Disable Relaxed JEDEC Reset during Late Command Training (Only for DDR4) 0:Disable, 1:Enable
Of course, if its value is never set, then it will very likely default to zero, and probably breaks things. This is definitely worth a try.
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37094 )
Change subject: Revert "3rdparts/fsp: Update fsp submodule" ......................................................................
Patch Set 2:
Patch Set 2:
Patch Set 2:
Patch Set 2:
How are we planing on dealing with this longterm? Reverting this would let fall Ice Lake out of support again... Could it be a temporary solution to keep the last known-good FSP.fd in the blobs repo for now and raise the problem with Intel or figure out the problem?
It might also be a quick solution to test the commit 9e53d779eb34e944f9b3386ad6a9df80f710bddd, that updated most but is before the f3ecfc496e9c07b553582197b5086cfe9948b384 commit "Coffee Lake FSP 7.0.68.40" and "Coffee Lake FSP 7.0.68.41" later that same day, so it still contains the CoffeLake FSP 7.0.64.40 Version from May 23rd.
I think i made a mistake.. Cannonlake is derived from Comet and not from CoffeeLake, right?
Cannonlake was meant to be the successor to Kaby Lake, with a shrink from 14nm to 10nm. However, AFAIUI, that shrink caused yield issues, and the only commercial Cannonlake chip is the i3-8121U. Instead, Coffee Lake was released, which uses a 14nm process.
I found this table particularly useful: https://en.wikipedia.org/wiki/Tick%E2%80%93tock_model#Roadmap
Patch Set 2:
Okay lets ease down here. I just double checked. What we could to is change this here - and instead of reverting it, just use commit 573bd5d6861376c8b4947d177dfe70592ff80fc2 (which is the renaming one) and use this as the submodule commit for now.
To be honest - I dont think it's my responsibility to debug the FSP now (how could I) - I am just notifying about the error - reverting this and we are go to go again.
Commit 573bd5d6861376c8b4947d177dfe70592ff80fc2 seems to work.
Alright, thanks for testing. This would narrow the issue down to three commits, one of which is just a documentation change. Therefore, the other two commits are extremely likely to be where the regression was introduced.
These last two commits correspond to the FSP 7.0.68.40 and FSP 7.0.68.41 updates for CFL. Do note that the former adds two additional FSP UPDs:
- LctRelaxedReset
- ScsSdCardWpPinEnabled
The latter is a FSP-S UPD related to the SD card controller's Write-Protect pin. I don't think it's much of an issue.
However, the other UPD is a FSP-M UPD, and is specific to DDR4 memory! The UPD description says:
Late Command Training Relaxed Reset Enable/Disable Relaxed JEDEC Reset during Late Command Training (Only for DDR4) 0:Disable, 1:Enable
Of course, if its value is never set, then it will very likely default to zero, and probably breaks things. This is definitely worth a try.
Okay - I did checked some things. So Commit f3ecfc496e9c07b553582197b5086cfe9948b384 (Update to version 7.0.68.40) is the commit which breaks the FSP. I tried setting the new introduced option 'LctRelaxedReset' in romstage/fsp_params.c to zero and one - none of those values work.
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37094 )
Change subject: Revert "3rdparts/fsp: Update fsp submodule" ......................................................................
Patch Set 2:
Patch Set 2:
Patch Set 2:
Patch Set 2:
Patch Set 2:
How are we planing on dealing with this longterm? Reverting this would let fall Ice Lake out of support again... Could it be a temporary solution to keep the last known-good FSP.fd in the blobs repo for now and raise the problem with Intel or figure out the problem?
It might also be a quick solution to test the commit 9e53d779eb34e944f9b3386ad6a9df80f710bddd, that updated most but is before the f3ecfc496e9c07b553582197b5086cfe9948b384 commit "Coffee Lake FSP 7.0.68.40" and "Coffee Lake FSP 7.0.68.41" later that same day, so it still contains the CoffeLake FSP 7.0.64.40 Version from May 23rd.
I think i made a mistake.. Cannonlake is derived from Comet and not from CoffeeLake, right?
Cannonlake was meant to be the successor to Kaby Lake, with a shrink from 14nm to 10nm. However, AFAIUI, that shrink caused yield issues, and the only commercial Cannonlake chip is the i3-8121U. Instead, Coffee Lake was released, which uses a 14nm process.
I found this table particularly useful: https://en.wikipedia.org/wiki/Tick%E2%80%93tock_model#Roadmap
Patch Set 2:
Okay lets ease down here. I just double checked. What we could to is change this here - and instead of reverting it, just use commit 573bd5d6861376c8b4947d177dfe70592ff80fc2 (which is the renaming one) and use this as the submodule commit for now.
To be honest - I dont think it's my responsibility to debug the FSP now (how could I) - I am just notifying about the error - reverting this and we are go to go again.
Commit 573bd5d6861376c8b4947d177dfe70592ff80fc2 seems to work.
Alright, thanks for testing. This would narrow the issue down to three commits, one of which is just a documentation change. Therefore, the other two commits are extremely likely to be where the regression was introduced.
These last two commits correspond to the FSP 7.0.68.40 and FSP 7.0.68.41 updates for CFL. Do note that the former adds two additional FSP UPDs:
- LctRelaxedReset
- ScsSdCardWpPinEnabled
The latter is a FSP-S UPD related to the SD card controller's Write-Protect pin. I don't think it's much of an issue.
However, the other UPD is a FSP-M UPD, and is specific to DDR4 memory! The UPD description says:
Late Command Training Relaxed Reset Enable/Disable Relaxed JEDEC Reset during Late Command Training (Only for DDR4) 0:Disable, 1:Enable
Of course, if its value is never set, then it will very likely default to zero, and probably breaks things. This is definitely worth a try.
Okay - I did checked some things. So Commit f3ecfc496e9c07b553582197b5086cfe9948b384 (Update to version 7.0.68.40) is the commit which breaks the FSP. I tried setting the new introduced option 'LctRelaxedReset' in romstage/fsp_params.c to zero and one - none of those values work.
I would suggest this patch here and merge CB 37669 instead.
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37094 )
Change subject: Revert "3rdparts/fsp: Update fsp submodule" ......................................................................
Patch Set 2:
Patch Set 2:
Patch Set 2:
Patch Set 2:
Patch Set 2:
Patch Set 2:
How are we planing on dealing with this longterm? Reverting this would let fall Ice Lake out of support again... Could it be a temporary solution to keep the last known-good FSP.fd in the blobs repo for now and raise the problem with Intel or figure out the problem?
It might also be a quick solution to test the commit 9e53d779eb34e944f9b3386ad6a9df80f710bddd, that updated most but is before the f3ecfc496e9c07b553582197b5086cfe9948b384 commit "Coffee Lake FSP 7.0.68.40" and "Coffee Lake FSP 7.0.68.41" later that same day, so it still contains the CoffeLake FSP 7.0.64.40 Version from May 23rd.
I think i made a mistake.. Cannonlake is derived from Comet and not from CoffeeLake, right?
Cannonlake was meant to be the successor to Kaby Lake, with a shrink from 14nm to 10nm. However, AFAIUI, that shrink caused yield issues, and the only commercial Cannonlake chip is the i3-8121U. Instead, Coffee Lake was released, which uses a 14nm process.
I found this table particularly useful: https://en.wikipedia.org/wiki/Tick%E2%80%93tock_model#Roadmap
Patch Set 2:
Okay lets ease down here. I just double checked. What we could to is change this here - and instead of reverting it, just use commit 573bd5d6861376c8b4947d177dfe70592ff80fc2 (which is the renaming one) and use this as the submodule commit for now.
To be honest - I dont think it's my responsibility to debug the FSP now (how could I) - I am just notifying about the error - reverting this and we are go to go again.
Commit 573bd5d6861376c8b4947d177dfe70592ff80fc2 seems to work.
Alright, thanks for testing. This would narrow the issue down to three commits, one of which is just a documentation change. Therefore, the other two commits are extremely likely to be where the regression was introduced.
These last two commits correspond to the FSP 7.0.68.40 and FSP 7.0.68.41 updates for CFL. Do note that the former adds two additional FSP UPDs:
- LctRelaxedReset
- ScsSdCardWpPinEnabled
The latter is a FSP-S UPD related to the SD card controller's Write-Protect pin. I don't think it's much of an issue.
However, the other UPD is a FSP-M UPD, and is specific to DDR4 memory! The UPD description says:
Late Command Training Relaxed Reset Enable/Disable Relaxed JEDEC Reset during Late Command Training (Only for DDR4) 0:Disable, 1:Enable
Of course, if its value is never set, then it will very likely default to zero, and probably breaks things. This is definitely worth a try.
Okay - I did checked some things. So Commit f3ecfc496e9c07b553582197b5086cfe9948b384 (Update to version 7.0.68.40) is the commit which breaks the FSP. I tried setting the new introduced option 'LctRelaxedReset' in romstage/fsp_params.c to zero and one - none of those values work.
I would suggest this patch here and merge CB 37669 instead.
I would suggest abandon this patch here and merge CB 37669 instead.
Christian Walter has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/37094 )
Change subject: Revert "3rdparts/fsp: Update fsp submodule" ......................................................................
Abandoned
-
Angel Pons (Code Review) -
Christian Walter (Code Review)
-
Mimoja (Code Review)
-
Patrick Georgi (Code Review)
-
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)