coreboot-gerrit May 2019

coreboot-gerrit@coreboot.org

1 participants
1485 discussions

Change in ...coreboot[master]: {arch,cpu}/x86, drivers/intel: Restore cpu_index error handling
by Jacob Garber (Code Review) 16 May '19

16 May '19

Jacob Garber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32795 Change subject: {arch,cpu}/x86, drivers/intel: Restore cpu_index error handling ...................................................................... {arch,cpu}/x86, drivers/intel: Restore cpu_index error handling Previously cpu_index() always succeeded, but since commit 095c931 (src/arch/x86: Use core apic id to get cpu_index()) it is now possible for it to indicate an error by returning -1. This commit adds error handling for all calls to cpu_index(), and restores several checks that were removed in 7c712bb (Fix code that would trip -Wtype-limits) but are now needed. Signed-off-by: Jacob Garber <jgarber1(a)ualberta.ca> Change-Id: I5436eed4cb5675f916924eb9670db04592a8b927 --- M src/arch/x86/cpu.c M src/arch/x86/exception.c M src/arch/x86/include/arch/cpu.h M src/cpu/x86/mp_init.c M src/cpu/x86/pae/pgtbl.c M src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c 6 files changed, 28 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/32795/1 diff --git a/src/arch/x86/cpu.c b/src/arch/x86/cpu.c index fb4c7b6..e6c9435 100644 --- a/src/arch/x86/cpu.c +++ b/src/arch/x86/cpu.c @@ -350,7 +350,7 @@ * Hence new logic to use cpuid to fetch lapic id and matches with * cpus_default_apic_id[] variable to return correct cpu_index(). */ -unsigned long cpu_index(void) +int cpu_index(void) { int i; int lapic_id = initial_lapicid(); diff --git a/src/arch/x86/exception.c b/src/arch/x86/exception.c index b00777a..b88f4a7 100644 --- a/src/arch/x86/exception.c +++ b/src/arch/x86/exception.c @@ -502,7 +502,7 @@ } #else /* !CONFIG_GDB_STUB */ #define MDUMP_SIZE 0x80 - unsigned int logical_processor = 0; + int logical_processor = 0; #if ENV_RAMSTAGE logical_processor = cpu_index(); diff --git a/src/arch/x86/include/arch/cpu.h b/src/arch/x86/include/arch/cpu.h index 481ee9d..078ea17 100644 --- a/src/arch/x86/include/arch/cpu.h +++ b/src/arch/x86/include/arch/cpu.h @@ -379,6 +379,6 @@ * Hence new logic to use cpuid to fetch lapic id and matches with * cpus_default_apic_id[] variable to return correct cpu_index(). */ -unsigned long cpu_index(void); +int cpu_index(void); #endif /* ARCH_CPU_H */ diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index 8957515..b7b8fe2 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -870,13 +870,20 @@ int i; int cpus_accepted; struct stopwatch sw; - int cur_cpu = cpu_index(); + int cur_cpu; if (!CONFIG(PARALLEL_MP_AP_WORK)) { printk(BIOS_ERR, "APs already parked. PARALLEL_MP_AP_WORK not selected.\n"); return -1; } + cur_cpu = cpu_index(); + + if (cur_cpu < 0) { + printk(BIOS_ERR, "Invalid CPU index.\n"); + return -1; + } + /* Signal to all the APs to run the func. */ for (i = 0; i < ARRAY_SIZE(ap_callbacks); i++) { if (cur_cpu == i) @@ -918,6 +925,12 @@ return; cur_cpu = cpu_index(); + + if (cur_cpu < 0) { + printk(BIOS_ERR, "Invalid CPU index.\n"); + return; + } + per_cpu_slot = &ap_callbacks[cur_cpu]; while (1) { diff --git a/src/cpu/x86/pae/pgtbl.c b/src/cpu/x86/pae/pgtbl.c index 062ee40..9c92134 100644 --- a/src/cpu/x86/pae/pgtbl.c +++ b/src/cpu/x86/pae/pgtbl.c @@ -116,12 +116,12 @@ static struct pg_table pgtbl[CONFIG_MAX_CPUS] __attribute__((aligned(4096))); static unsigned long mapped_window[CONFIG_MAX_CPUS]; - unsigned long index; + int index; unsigned long window; void *result; int i; index = cpu_index(); - if (index >= CONFIG_MAX_CPUS) + if (index < 0) return MAPPING_ERROR; window = page >> 10; if (window != mapped_window[index]) { diff --git a/src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c b/src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c index cdc98e0..31d75a5 100644 --- a/src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c +++ b/src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c @@ -45,6 +45,9 @@ efi_uintn_t processor_number, efi_processor_information *processor_info_buffer) { + if (cpu_index() < 0) + return FSP_DEVICE_ERROR; + if (processor_info_buffer == NULL) return FSP_INVALID_PARAMETER; @@ -68,6 +71,9 @@ efi_ap_procedure procedure, efi_boolean_t ignored3, efi_uintn_t timeout_usec, void *argument) { + if (cpu_index() < 0) + return FSP_DEVICE_ERROR; + if (procedure == NULL) return FSP_INVALID_PARAMETER; @@ -85,6 +91,9 @@ efi_ap_procedure procedure, efi_uintn_t processor_number, efi_uintn_t timeout_usec, void *argument) { + if (cpu_index() < 0) + return FSP_DEVICE_ERROR; + if (processor_number > get_cpu_count()) return FSP_NOT_FOUND; -- To view, visit https://review.coreboot.org/c/coreboot/+/32795 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5436eed4cb5675f916924eb9670db04592a8b927 Gerrit-Change-Number: 32795 Gerrit-PatchSet: 1 Gerrit-Owner: Jacob Garber <jgarber1(a)ualberta.ca> Gerrit-MessageType: newchange

3 6

Change in ...coreboot[master]: Documentation/lessons: Tidy up lesson 2
by Jacob Garber (Code Review) 16 May '19

16 May '19

Jacob Garber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32804 Change subject: Documentation/lessons: Tidy up lesson 2 ...................................................................... Documentation/lessons: Tidy up lesson 2 - The link to create an account is now "Sign in" and not "Register" - Use monospace formatting for terminal commands and file names - Properly escape less-than and greater-than - Correct the 'make lint' example command - Reformat example commit messages - Add formatting for website links - Other whitespace fixes Signed-off-by: Jacob Garber <jgarber1(a)ualberta.ca> Change-Id: I9931bef8c30387d1c08b59973d6de9b5c0419814 --- M Documentation/lessons/lesson2.md 1 file changed, 52 insertions(+), 44 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/32804/1 diff --git a/Documentation/lessons/lesson2.md b/Documentation/lessons/lesson2.md index d6f800c..b952280 100644 --- a/Documentation/lessons/lesson2.md +++ b/Documentation/lessons/lesson2.md @@ -5,10 +5,10 @@ If you already have an account, skip to Part 2. Otherwise, go to <https://review.coreboot.org> in your preferred web browser. -Select **Register** in the upper right corner. +Select **Sign in** in the upper right corner. Select the appropriate sign-in. For example, if you have a Google account, -select **Google OAuth2** (gerrit-oauth-provider plugin)".**Note:** Your +select **Google OAuth2** (gerrit-oauth-provider plugin). **Note:** Your username for the account will be the username of the account you used to sign-in with. (ex. your Google username). @@ -17,7 +17,7 @@ If you prefer to use an HTTP password instead, skip to Part 2b. For the most up-to-date instructions on how to set up SSH keys with Gerrit go to -<https://gerrit-documentation.storage.googleapis.com/Documentation/2.14.2/us…)> +<https://gerrit-documentation.storage.googleapis.com/Documentation/2.14.2/us…> and follow the instructions there. Then, skip to Part 3. Additionally, that section of the Web site provides explanation on starting @@ -35,13 +35,13 @@ In the upper right corner, select your name and click on **Settings**. Select **SSH Public Keys** on the left-hand side. -In a terminal, run "ssh-keygen" and confirm the default path ".ssh/id_rsa". +In a terminal, run `ssh-keygen` and confirm the default path `.ssh/id_rsa`. Make a passphrase -- remember this phrase. It will be needed whenever you use this RSA Public Key. **Note:** You might want to use a short password, or forego the password altogether as you will be using it very often. -Open "id_rsa.pub", copy all contents and paste into the textbox under +Open `id_rsa.pub`, copy all contents and paste into the textbox under "Add SSH Public Key" in the https://review.coreboot.org webpage. ## Part 2b: Setting up an HTTP Password @@ -51,7 +51,7 @@ than selecting **SSH Public Keys**, select **HTTP Password**. Click **Generate Password**. This should fill the "Password" box with a password. Copy -the password, and add the following to your $HOME/.netrc file: +the password, and add the following to your `$HOME/.netrc` file: machine review.coreboot.org login YourUserNameHere password YourPasswordHere @@ -61,7 +61,7 @@ ## Part 3: Clone coreboot and configure it for submitting patches On Gerrit, click on the **Browse** tab in the upper left corner and select -**Repositories**. From the listing, select the "coreboot" repo. You may have +**Repositories**. From the listing, select the "coreboot" repo. You may have to click the next page arrow at the bottom a few times to find it. If you are using SSH keys, select **ssh** from the tabs under "Project @@ -69,7 +69,7 @@ This should prompt you for your id_rsa passphrase, if you previously set one. If you are using HTTP, instead, select **http** from the tabs under "Project coreboot" -and run the command that appears +and run the command that appears. Now is a good time to configure your global git identity, if you haven't already. @@ -87,13 +87,13 @@ An easy first commit to make is fixing existing checkpatch errors and warnings in the source files. To see errors that are already present, build the files in -the repository by running 'make lint' in the coreboot directory. Alternatively, -if you want to run 'make lint' on a specific directory, run: +the repository by running `make lint` in the coreboot directory. Alternatively, +if you want to run `make lint` on a specific directory, run: - for file in $(git ls-files | grep src/amd/quadcore); do \ + for file in $(git ls-files | grep <filepath>); do \ util/lint/checkpatch.pl --file $file --terse; done -where <filepath> is the filepath of the directory (ex. src/cpu/amd/car). +where `filepath` is the filepath of the directory (ex. `src/cpu/amd/car`). Any changes made to files under the src directory are made locally, and can be submitted for review. @@ -116,7 +116,7 @@ git commit -s **Note:** The -s adds a signed-off-by line by the committer. Your commit should be -signed off with your name and email (i.e. **Your Name** **<Your Email>**, based on +signed off with your name and email (i.e. **Your Name** **\<Your Email\>**, based on what you set with git config earlier). Running git commit first checks for any errors and warnings using lint. If @@ -130,65 +130,73 @@ one-line description of what you changed in the files using the template below: -`<filepath>: Short description` -*ex. cpu/amd/pi/00630F01: Fix checkpatch warnings and errors* + <filepath>: Short description + +For example, + + cpu/amd/pi/00630F01: Fix checkpatch warnings and errors **Note:** It is good practice to use present tense in your descriptions and do not punctuate your summary. Then hit Enter. The next paragraph should be a more in-depth explanation of the changes you've made to the files. Again, it is good practice to use present -tense. -*ex. Fix space prohibited between function name and open parenthesis, -line over 80 characters, unnecessary braces for single statement blocks, -space required before open brace errors and warnings.* +tense. Ex. + + Fix space prohibited between function name and open parenthesis, + line over 80 characters, unnecessary braces for single statement blocks, + space required before open brace errors and warnings. When you have finished writing your commit message, save and exit the text editor. You have finished committing your change. If, after submitting your -commit, you wish to make changes to it, running "git commit --amend" allows +commit, you wish to make changes to it, running `git commit --amend` allows you to take back your commit and amend it. -When you are done with your commit, run 'git push' to push your commit to +When you are done with your commit, run `git push` to push your commit to coreboot.org. **Note:** To submit as a draft, use -'git push origin HEAD:refs/drafts/master' Submitting as a draft means that +`git push origin HEAD:refs/drafts/master`. Submitting as a draft means that your commit will be on coreboot.org, but is only visible to those you add as reviewers. This has been a quick primer on how to submit a change to Gerrit for review -using git. You may wish to review the [Gerrit code review workflow +using git. You may wish to review the [Gerrit code review workflow documentation](https://gerrit-review.googlesource.com/Documentation/intro-u…, especially if you plan to work on multiple changes at the same time. ## Part 4b: Using git cola to stage and submit a commit If git cola is not installed on your machine, see -https://git-cola.github.io/downloads.html for download instructions. +<https://git-cola.github.io/downloads.html> for download instructions. -After making some edits to src files, rather than run "git add," run -'git cola' from the command line. You should see all of the files +After making some edits to src files, rather than run `git add`, run +`git cola` from the command line. You should see all of the files edited under "Modified". In the textbox labeled "Commit summary" provide a brief one-line description of what you changed in the files according to the template below: -`<filepath>: Short description` -*ex. cpu/amd/pi/00630F01: Fix checkpatch warnings and errors* + <filepath>: Short description + +For example, + + cpu/amd/pi/00630F01: Fix checkpatch warnings and errors **Note:** It is good practice to use present tense in your descriptions and do not punctuate your short description. In the larger text box labeled 'Extended description...' provide a more in-depth explanation of the changes you've made to the files. Again, it -is good practice to use present tense. -*ex. Fix space prohibited between function name and open parenthesis, -line over 80 characters, unnecessary braces for single statement blocks, -space required before open brace errors and warnings.* +is good practice to use present tense. Ex. + + Fix space prohibited between function name and open parenthesis, + line over 80 characters, unnecessary braces for single statement blocks, + space required before open brace errors and warnings. Then press Enter two times to move the cursor to below your description. To the left of the text boxes, there is an icon with an downward arrow. Press the arrow and select "Sign Off." Make sure that you are signing off -with your name and email (i.e. **Your Name** **<Your Email>**, based on what +with your name and email (i.e. **Your Name** **\<Your Email\>**, based on what you set with git config earlier). Now, review each of your changes and mark either individual changes or @@ -214,30 +222,30 @@ explained in the extended description. When ready, select 'Commit' again. Once all errors have been satisfied -and the commit succeeds, move to the command line and run 'git push'. -**Note:** To submit as a draft, use 'git push origin HEAD:refs/drafts/master' +and the commit succeeds, move to the command line and run `git push`. +**Note:** To submit as a draft, use `git push origin HEAD:refs/drafts/master`. Submitting as a draft means that your commit will be on coreboot.org, but is only visible to those you add as reviewers. ## Part 5: Getting your commit reviewed -Your commits can now be seen on review.coreboot.org if you select “Your” -and click on “Changes” and can be reviewed by others. Your code will +Your commits can now be seen on review.coreboot.org if you select "Your" +and click on "Changes" and can be reviewed by others. Your code will first be reviewed by build bot (Jenkins), which will either give you a warning or verify a successful build; if so, your commit will receive a +1. Other -users may also give your commit +1. For a commit to be merged, it needs -to receive a +2.**Note:** A +1 and a +1 does not make a +2. Only certain users +users may also give your commit +1. For a commit to be merged, it needs +to receive a +2. **Note:** A +1 and a +1 does not make a +2. Only certain users can give a +2. ## Part 6 (optional): bash-git-prompt To help make it easier to understand the state of the git repository -without running 'git status' or 'git log', there is a way to make the +without running `git status` or `git log`, there is a way to make the command line show the status of the repository at every point. This is through bash-git-prompt. Instructions for installing this are found at: -https://github.com/magicmonty/bash-git-prompt +<https://github.com/magicmonty/bash-git-prompt>. **Note:** Feel free to search for different versions of git prompt, as this one is specific to bash. @@ -250,7 +258,7 @@ **Note:** cd will change your directory to your home directory, so the git clone command will be run there. -Finally, open the ~/.bashrc file and append the following two lines: +Finally, open the `~/.bashrc` file and append the following two lines: GIT_PROMPT_ONLY_IN_REPO=1 source ~/.bash-git-prompt/gitprompt.sh @@ -260,7 +268,7 @@ There also are additional configurations that you can change depending on your preferences. If you wish to do so, look at the "All configs for .bashrc" section -on https://github.com/magicmonty/bash-git-prompt. Listed in that section are +on <https://github.com/magicmonty/bash-git-prompt>. Listed in that section are various lines that you can copy, uncomment and add to your .bashrc file to change the configurations. Example configurations include avoid fetching remote status, and supporting versions of Git older than 1.7.10. @@ -273,7 +281,7 @@ remote repository. If the commit you wish to update is the most recent commit you have made, after making your desired changes, stage the files (either using git add or in git cola), and amend the commit. To do so, -if you are using the command line, run "git commit --amend." If you are +if you are using the command line, run `git commit --amend`. If you are using git cola, click on the gear icon located on the upper left side under **Commit** and select **Amend Last Commit** in the drop down menu. Then, stage the files you have changed, commit the changes, and run git push to push the -- To view, visit https://review.coreboot.org/c/coreboot/+/32804 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9931bef8c30387d1c08b59973d6de9b5c0419814 Gerrit-Change-Number: 32804 Gerrit-PatchSet: 1 Gerrit-Owner: Jacob Garber <jgarber1(a)ualberta.ca> Gerrit-MessageType: newchange

5 16

Change in ...coreboot[master]: mb/google/sarien: leave gpio pads unlocks during fsp
by Jett Rink (Code Review) 16 May '19

16 May '19

Jett Rink has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32831 Change subject: mb/google/sarien: leave gpio pads unlocks during fsp ...................................................................... mb/google/sarien: leave gpio pads unlocks during fsp The FSP will lock down the configuration of GPP_A12, which makes the configuration of the GPIO pin on warm reset not work correctly. This is only needed for the Arcada variant since it is the only variant that uses ISH. BRANCH=sarien BUG=b:132719369 TEST=ISH_GP6 now works on warm resets on arcarda Change-Id: Icb3bae2c48eee053189f1a878f5975c6afe51c71 Signed-off-by: Jett Rink <jettrink(a)chromium.org> --- M src/mainboard/google/sarien/variants/arcada/devicetree.cb 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/32831/1 diff --git a/src/mainboard/google/sarien/variants/arcada/devicetree.cb b/src/mainboard/google/sarien/variants/arcada/devicetree.cb index 27c61f3..b6377ba 100644 --- a/src/mainboard/google/sarien/variants/arcada/devicetree.cb +++ b/src/mainboard/google/sarien/variants/arcada/devicetree.cb @@ -26,6 +26,7 @@ register "PchPmSlpS4MinAssert" = "4" # 4s register "PchPmSlpSusMinAssert" = "4" # 4s register "PchPmSlpAMinAssert" = "4" # 2s + register "PchUnlockGpioPads" = "1" register "speed_shift_enable" = "1" register "psys_pmax" = "140" -- To view, visit https://review.coreboot.org/c/coreboot/+/32831 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Icb3bae2c48eee053189f1a878f5975c6afe51c71 Gerrit-Change-Number: 32831 Gerrit-PatchSet: 1 Gerrit-Owner: Jett Rink <jettrink(a)chromium.org> Gerrit-MessageType: newchange

3 4

Change in ...coreboot[master]: {drivers,soc/intel/braswell}: Add C_ENVIRONMENT_BOOTBLOCK support
by Frans Hendriks (Code Review) 16 May '19

16 May '19

Hello Patrick Rudolph, Huang Jin, Arthur Heymans, York Yang, Lee Leahy, Matt DeVillier, build bot (Jenkins), Hannah Williams, Michał Żygowski, Patrick Georgi, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/29662 to look at the new patch set (#22). Change subject: {drivers,soc/intel/braswell}: Add C_ENVIRONMENT_BOOTBLOCK support ...................................................................... {drivers,soc/intel/braswell}: Add C_ENVIRONMENT_BOOTBLOCK support No C_ENVIRONMENT_BOOTBLOCK support for Braswell is available. Enable support and add required files for the Braswell Bootblock in C. The next changes are made support C_ENVIRONMENT_BOOTBLOCK: - Add post init console functions romstage_c_entry() . - Add car_stage_entry() function bootblock-c_entry() functions. - Specify config DCACHE_BSP_STACK_SIZE and C_ENV_BOOTBLOCK_SIZE. - Add bootblock_c_entry(). Removed the unused cache_as_ram_main(). BUG=NA TEST=Booting Embedded Linux on Facebook FBG-1701 Building Google Banos Change-Id: Iab48ad72f1514c93f20d70db5ef4fd8fa2383e8c Signed-off-by: Frans Hendriks <fhendriks(a)eltan.com> --- M src/drivers/intel/fsp1_1/Makefile.inc M src/drivers/intel/fsp1_1/car.c M src/drivers/intel/fsp1_1/include/fsp/car.h M src/soc/intel/braswell/Kconfig M src/soc/intel/braswell/Makefile.inc M src/soc/intel/braswell/bootblock/bootblock.c R src/soc/intel/braswell/bootblock/cache_as_ram.S M src/soc/intel/braswell/romstage/Makefile.inc A src/soc/intel/braswell/romstage/car_stage_entry.S 9 files changed, 88 insertions(+), 168 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/29662/22 -- To view, visit https://review.coreboot.org/c/coreboot/+/29662 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iab48ad72f1514c93f20d70db5ef4fd8fa2383e8c Gerrit-Change-Number: 29662 Gerrit-PatchSet: 22 Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Huang Jin <huang.jin(a)intel.com> Gerrit-Reviewer: Lee Leahy <leroy.p.leahy(a)intel.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: York Yang <yyang024(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newpatchset

1 0

Change in ...coreboot[master]: mainboard/facebook/fbg1701: Do initial mainboard commit
by build bot (Jenkins) (Code Review) 16 May '19

16 May '19

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30414 ) Change subject: mainboard/facebook/fbg1701: Do initial mainboard commit ...................................................................... Patch Set 23: (2 comments) https://review.coreboot.org/#/c/30414/23/src/mainboard/facebook/fbg1701/irq… File src/mainboard/facebook/fbg1701/irqroute.h: https://review.coreboot.org/#/c/30414/23/src/mainboard/facebook/fbg1701/irq… PS23, Line 40: #define PCI_DEV_PIRQ_ROUTES \ Macros with complex values should be enclosed in parentheses https://review.coreboot.org/#/c/30414/23/src/mainboard/facebook/fbg1701/irq… PS23, Line 62: #define PIRQ_PIC_ROUTES \ Macros with complex values should be enclosed in parentheses -- To view, visit https://review.coreboot.org/c/coreboot/+/30414 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I28ac78a630ee705b1e546031f024bfe7f952ab39 Gerrit-Change-Number: 30414 Gerrit-PatchSet: 23 Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: David Hendricks <david.hendricks(a)gmail.com> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Comment-Date: Thu, 16 May 2019 10:17:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: mainboard/facebook/fbg1701: Do initial mainboard commit
by Frans Hendriks (Code Review) 16 May '19

16 May '19

Hello Patrick Rudolph, build bot (Jenkins), Nico Huber, Michał Żygowski, Patrick Georgi, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/30414 to look at the new patch set (#23). Change subject: mainboard/facebook/fbg1701: Do initial mainboard commit ...................................................................... mainboard/facebook/fbg1701: Do initial mainboard commit Initial support for Facebook FBG-1701 system. coreboot implementation based on Intel Strago mainboard. Configure 'Onboard memory manufacture' which must match HW. BUG=N/A TEST=booting SeaBIOS and Linux 4.15+ kernel on Facebook FBG-1701 Change-Id: I28ac78a630ee705b1e546031f024bfe7f952ab39 Signed-off-by: Frans Hendriks <fhendriks(a)eltan.com> --- A Documentation/mainboard/facebook/fbg1701.md M Documentation/mainboard/index.md A src/mainboard/facebook/fbg1701/Kconfig A src/mainboard/facebook/fbg1701/Kconfig.name A src/mainboard/facebook/fbg1701/Makefile.inc A src/mainboard/facebook/fbg1701/acpi/ec.asl A src/mainboard/facebook/fbg1701/acpi/mainboard.asl A src/mainboard/facebook/fbg1701/acpi/sleepstates.asl A src/mainboard/facebook/fbg1701/acpi/superio.asl A src/mainboard/facebook/fbg1701/acpi_tables.c A src/mainboard/facebook/fbg1701/board_info.txt A src/mainboard/facebook/fbg1701/bootblock.c A src/mainboard/facebook/fbg1701/cmos.layout A src/mainboard/facebook/fbg1701/com_init.c A src/mainboard/facebook/fbg1701/devicetree.cb A src/mainboard/facebook/fbg1701/dsdt.asl A src/mainboard/facebook/fbg1701/fadt.c A src/mainboard/facebook/fbg1701/gpio.c A src/mainboard/facebook/fbg1701/hda_verb.c A src/mainboard/facebook/fbg1701/irqroute.c A src/mainboard/facebook/fbg1701/irqroute.h A src/mainboard/facebook/fbg1701/logo.c A src/mainboard/facebook/fbg1701/mainboard.c A src/mainboard/facebook/fbg1701/mainboard.h A src/mainboard/facebook/fbg1701/onboard.h A src/mainboard/facebook/fbg1701/ramstage.c A src/mainboard/facebook/fbg1701/romstage.c A src/mainboard/facebook/fbg1701/spd/MICRON_MT41K512M16HA-125A.spd.hex A src/mainboard/facebook/fbg1701/spd/SAMSUNG_K4B8G1646D-MYKO.spd.hex A src/mainboard/facebook/fbg1701/w25q64.c 30 files changed, 2,078 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/30414/23 -- To view, visit https://review.coreboot.org/c/coreboot/+/30414 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I28ac78a630ee705b1e546031f024bfe7f952ab39 Gerrit-Change-Number: 30414 Gerrit-PatchSet: 23 Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: David Hendricks <david.hendricks(a)gmail.com> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in ...coreboot[master]: soc/amd/stoneyridge: Move I2C bus clear out of gpio.c
by Marshall Dawson (Code Review) 16 May '19

16 May '19

Hello Richard Spiegel, Martin Roth, Furquan Shaikh, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/32650 to review the following change. Change subject: soc/amd/stoneyridge: Move I2C bus clear out of gpio.c ...................................................................... soc/amd/stoneyridge: Move I2C bus clear out of gpio.c Relocate the I2C bus reset code from gpio.c to i2c.c. When it first went in, gpio.c was a natural location due to the nature of the algorithm. This is preparation for moving most of gpio.c to common code. Change-Id: I3b2d8e1b54e7c5929220d763bd99fe01b0636aaa Signed-off-by: Marshall Dawson <marshalldawson3rd(a)gmail.com> --- M src/soc/amd/stoneyridge/chip.h M src/soc/amd/stoneyridge/gpio.c M src/soc/amd/stoneyridge/i2c.c M src/soc/amd/stoneyridge/include/soc/gpio.h A src/soc/amd/stoneyridge/include/soc/i2c.h 5 files changed, 149 insertions(+), 124 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/32650/1 diff --git a/src/soc/amd/stoneyridge/chip.h b/src/soc/amd/stoneyridge/chip.h index 92223d1..d1a7d30 100644 --- a/src/soc/amd/stoneyridge/chip.h +++ b/src/soc/amd/stoneyridge/chip.h @@ -20,7 +20,7 @@ #include <stdint.h> #include <commonlib/helpers.h> #include <drivers/i2c/designware/dw_i2c.h> -#include <soc/gpio.h> +#include <soc/i2c.h> #include <arch/acpi_device.h> #define MAX_NODES 1 diff --git a/src/soc/amd/stoneyridge/gpio.c b/src/soc/amd/stoneyridge/gpio.c index 01ced08..7c96805 100644 --- a/src/soc/amd/stoneyridge/gpio.c +++ b/src/soc/amd/stoneyridge/gpio.c @@ -18,11 +18,9 @@ #include <device/mmio.h> #include <device/device.h> #include <console/console.h> -#include <delay.h> #include <gpio.h> #include <amdblocks/acpimmio.h> #include <soc/gpio.h> -#include <soc/pci_devs.h> #include <assert.h> #include "chip.h" @@ -326,101 +324,6 @@ edge_level, mask); } -/* - * I2C pins are open drain with external pull up, so in order to bit bang them - * all, SCL pins must become GPIO inputs with no pull, then they need to be - * toggled between input-no-pull and output-low. This table is for the initial - * conversion of all SCL pins to input with no pull. - */ -static const struct soc_amd_gpio i2c_2_gpi[] = { - PAD_GPI(I2C0_SCL_PIN, PULL_NONE), - PAD_GPI(I2C1_SCL_PIN, PULL_NONE), - PAD_GPI(I2C2_SCL_PIN, PULL_NONE), - PAD_GPI(I2C3_SCL_PIN, PULL_NONE), -}; -#define saved_pins_count ARRAY_SIZE(i2c_2_gpi) - -/* - * To program I2C pins without destroying their programming, the registers - * that will be changed need to be saved first. - */ -static void save_i2c_pin_registers(uint8_t gpio, - struct soc_amd_i2c_save *save_table) -{ - uint32_t *gpio_ptr; - - gpio_ptr = (uint32_t *)gpio_get_address(gpio); - save_table->mux_value = iomux_read8(gpio); - save_table->control_value = read32(gpio_ptr); -} - -static void restore_i2c_pin_registers(uint8_t gpio, - struct soc_amd_i2c_save *save_table) -{ - uint32_t *gpio_ptr; - - gpio_ptr = (uint32_t *)gpio_get_address(gpio); - iomux_write8(gpio, save_table->mux_value); - iomux_read8(gpio); - write32(gpio_ptr, save_table->control_value); - read32(gpio_ptr); -} - -/* Slaves to be reset are controlled by devicetree register i2c_scl_reset */ -void sb_reset_i2c_slaves(void) -{ - const struct soc_amd_stoneyridge_config *cfg; - const struct device *dev = pcidev_path_on_root(GNB_DEVFN); - struct soc_amd_i2c_save save_table[saved_pins_count]; - uint8_t i, j, control; - - if (!dev || !dev->chip_info) - return; - cfg = dev->chip_info; - control = cfg->i2c_scl_reset & GPIO_I2C_MASK; - if (control == 0) - return; - - /* Save and reprogram I2C SCL pins */ - for (i = 0; i < saved_pins_count; i++) - save_i2c_pin_registers(i2c_2_gpi[i].gpio, &save_table[i]); - sb_program_gpios(i2c_2_gpi, saved_pins_count); - - /* - * Toggle SCL back and forth 9 times under 100KHz. A single read is - * needed after the writes to force the posted write to complete. - */ - for (j = 0; j < 9; j++) { - if (control & GPIO_I2C0_SCL) - write32((uint32_t *)GPIO_I2C0_ADDRESS, GPIO_SCL_LOW); - if (control & GPIO_I2C1_SCL) - write32((uint32_t *)GPIO_I2C1_ADDRESS, GPIO_SCL_LOW); - if (control & GPIO_I2C2_SCL) - write32((uint32_t *)GPIO_I2C2_ADDRESS, GPIO_SCL_LOW); - if (control & GPIO_I2C3_SCL) - write32((uint32_t *)GPIO_I2C3_ADDRESS, GPIO_SCL_LOW); - - read32((uint32_t *)GPIO_I2C3_ADDRESS); /* Flush posted write */ - udelay(4); /* 4usec gets 85KHz for 1 pin, 70KHz for 4 pins */ - - if (control & GPIO_I2C0_SCL) - write32((uint32_t *)GPIO_I2C0_ADDRESS, GPIO_SCL_HIGH); - if (control & GPIO_I2C1_SCL) - write32((uint32_t *)GPIO_I2C1_ADDRESS, GPIO_SCL_HIGH); - if (control & GPIO_I2C2_SCL) - write32((uint32_t *)GPIO_I2C2_ADDRESS, GPIO_SCL_HIGH); - if (control & GPIO_I2C3_SCL) - write32((uint32_t *)GPIO_I2C3_ADDRESS, GPIO_SCL_HIGH); - - read32((uint32_t *)GPIO_I2C3_ADDRESS); /* Flush posted write */ - udelay(4); - } - - /* Restore I2C pins. */ - for (i = 0; i < saved_pins_count; i++) - restore_i2c_pin_registers(i2c_2_gpi[i].gpio, &save_table[i]); -} - int gpio_interrupt_status(gpio_t gpio) { uintptr_t gpio_address = gpio_get_address(gpio); diff --git a/src/soc/amd/stoneyridge/i2c.c b/src/soc/amd/stoneyridge/i2c.c index e99f3509f..c3e5539 100644 --- a/src/soc/amd/stoneyridge/i2c.c +++ b/src/soc/amd/stoneyridge/i2c.c @@ -13,12 +13,16 @@ * GNU General Public License for more details. */ +#include <device/mmio.h> #include <arch/acpi.h> #include <console/console.h> +#include <delay.h> #include <drivers/i2c/designware/dw_i2c.h> +#include <amdblocks/acpimmio.h> #include <soc/iomap.h> #include <soc/pci_devs.h> #include <soc/southbridge.h> +#include <soc/i2c.h> #include "chip.h" #define I2C_BUS_ADDRESS(x) (I2C_BASE_ADDRESS + I2C_DEVICE_SIZE * (x)) @@ -140,3 +144,98 @@ .acpi_name = i2c_acpi_name, .acpi_fill_ssdt_generator = dw_i2c_acpi_fill_ssdt, }; + +/* + * I2C pins are open drain with external pull up, so in order to bit bang them + * all, SCL pins must become GPIO inputs with no pull, then they need to be + * toggled between input-no-pull and output-low. This table is for the initial + * conversion of all SCL pins to input with no pull. + */ +static const struct soc_amd_gpio i2c_2_gpi[] = { + PAD_GPI(I2C0_SCL_PIN, PULL_NONE), + PAD_GPI(I2C1_SCL_PIN, PULL_NONE), + PAD_GPI(I2C2_SCL_PIN, PULL_NONE), + PAD_GPI(I2C3_SCL_PIN, PULL_NONE), +}; +#define saved_pins_count ARRAY_SIZE(i2c_2_gpi) + +/* + * To program I2C pins without destroying their programming, the registers + * that will be changed need to be saved first. + */ +static void save_i2c_pin_registers(uint8_t gpio, + struct soc_amd_i2c_save *save_table) +{ + uint32_t *gpio_ptr; + + gpio_ptr = (uint32_t *)gpio_get_address(gpio); + save_table->mux_value = iomux_read8(gpio); + save_table->control_value = read32(gpio_ptr); +} + +static void restore_i2c_pin_registers(uint8_t gpio, + struct soc_amd_i2c_save *save_table) +{ + uint32_t *gpio_ptr; + + gpio_ptr = (uint32_t *)gpio_get_address(gpio); + iomux_write8(gpio, save_table->mux_value); + iomux_read8(gpio); + write32(gpio_ptr, save_table->control_value); + read32(gpio_ptr); +} + +/* Slaves to be reset are controlled by devicetree register i2c_scl_reset */ +void sb_reset_i2c_slaves(void) +{ + const struct soc_amd_stoneyridge_config *cfg; + const struct device *dev = pcidev_path_on_root(GNB_DEVFN); + struct soc_amd_i2c_save save_table[saved_pins_count]; + uint8_t i, j, control; + + if (!dev || !dev->chip_info) + return; + cfg = dev->chip_info; + control = cfg->i2c_scl_reset & GPIO_I2C_MASK; + if (control == 0) + return; + + /* Save and reprogram I2C SCL pins */ + for (i = 0; i < saved_pins_count; i++) + save_i2c_pin_registers(i2c_2_gpi[i].gpio, &save_table[i]); + sb_program_gpios(i2c_2_gpi, saved_pins_count); + + /* + * Toggle SCL back and forth 9 times under 100KHz. A single read is + * needed after the writes to force the posted write to complete. + */ + for (j = 0; j < 9; j++) { + if (control & GPIO_I2C0_SCL) + write32((uint32_t *)GPIO_I2C0_ADDRESS, GPIO_SCL_LOW); + if (control & GPIO_I2C1_SCL) + write32((uint32_t *)GPIO_I2C1_ADDRESS, GPIO_SCL_LOW); + if (control & GPIO_I2C2_SCL) + write32((uint32_t *)GPIO_I2C2_ADDRESS, GPIO_SCL_LOW); + if (control & GPIO_I2C3_SCL) + write32((uint32_t *)GPIO_I2C3_ADDRESS, GPIO_SCL_LOW); + + read32((uint32_t *)GPIO_I2C3_ADDRESS); /* Flush posted write */ + udelay(4); /* 4usec gets 85KHz for 1 pin, 70KHz for 4 pins */ + + if (control & GPIO_I2C0_SCL) + write32((uint32_t *)GPIO_I2C0_ADDRESS, GPIO_SCL_HIGH); + if (control & GPIO_I2C1_SCL) + write32((uint32_t *)GPIO_I2C1_ADDRESS, GPIO_SCL_HIGH); + if (control & GPIO_I2C2_SCL) + write32((uint32_t *)GPIO_I2C2_ADDRESS, GPIO_SCL_HIGH); + if (control & GPIO_I2C3_SCL) + write32((uint32_t *)GPIO_I2C3_ADDRESS, GPIO_SCL_HIGH); + + read32((uint32_t *)GPIO_I2C3_ADDRESS); /* Flush posted write */ + udelay(4); + } + + /* Restore I2C pins. */ + for (i = 0; i < saved_pins_count; i++) + restore_i2c_pin_registers(i2c_2_gpi[i].gpio, &save_table[i]); +} diff --git a/src/soc/amd/stoneyridge/include/soc/gpio.h b/src/soc/amd/stoneyridge/include/soc/gpio.h index fe8240f..26d0336 100644 --- a/src/soc/amd/stoneyridge/include/soc/gpio.h +++ b/src/soc/amd/stoneyridge/include/soc/gpio.h @@ -36,21 +36,10 @@ uint8_t event; }; -struct soc_amd_i2c_save { - uint32_t control_value; - uint8_t mux_value; -}; - #define GPIO_MASTER_SWITCH 0xFC #define GPIO_MASK_STS_EN BIT(28) #define GPIO_INTERRUPT_EN BIT(30) -#define GPIO_I2C0_SCL BIT(0) -#define GPIO_I2C1_SCL BIT(1) -#define GPIO_I2C2_SCL BIT(2) -#define GPIO_I2C3_SCL BIT(3) -#define GPIO_I2C_MASK (BIT(0) | BIT(1) | BIT(2) | BIT(3)) - #define GPIO_TOTAL_PINS 149 #define GPIO_PIN_IN (1 << 0) /* for byte access */ #define GPIO_PIN_OUT (1 << 6) /* for byte access */ @@ -186,15 +175,6 @@ #define GPIO_147 147 #define GPIO_148 148 -#define I2C0_SCL_PIN GPIO_145 -#define I2C1_SCL_PIN GPIO_147 -#define I2C2_SCL_PIN GPIO_113 -#define I2C3_SCL_PIN GPIO_19 - -#define GPIO_I2C0_ADDRESS GPIO_BANK2_CONTROL(I2C0_SCL_PIN) -#define GPIO_I2C1_ADDRESS GPIO_BANK2_CONTROL(I2C1_SCL_PIN) -#define GPIO_I2C2_ADDRESS GPIO_BANK1_CONTROL(I2C2_SCL_PIN) -#define GPIO_I2C3_ADDRESS GPIO_BANK0_CONTROL(I2C3_SCL_PIN) #define GPIO_SCL_HIGH 0 #define GPIO_SCL_LOW GPIO_OUTPUT_ENABLE @@ -373,11 +353,6 @@ #define GPIO_148_IOMUX_I2C1_SDA 0 #define GPIO_148_IOMUX_GPIOxx 1 -#define I2C0_SCL_PIN_IOMUX_GPIOxx GPIO_145_IOMUX_GPIOxx -#define I2C1_SCL_PIN_IOMUX_GPIOxx GPIO_147_IOMUX_GPIOxx -#define I2C2_SCL_PIN_IOMUX_GPIOxx GPIO_113_IOMUX_GPIOxx -#define I2C3_SCL_PIN_IOMUX_GPIOxx GPIO_19_IOMUX_GPIOxx - enum { GEVENT_0, GEVENT_1, @@ -599,7 +574,6 @@ * @return none */ void sb_program_gpios(const struct soc_amd_gpio *gpio_list_ptr, size_t size); -void sb_reset_i2c_slaves(void); /* Return the interrupt status and clear if set. */ int gpio_interrupt_status(gpio_t gpio); diff --git a/src/soc/amd/stoneyridge/include/soc/i2c.h b/src/soc/amd/stoneyridge/include/soc/i2c.h new file mode 100644 index 0000000..62575d0 --- /dev/null +++ b/src/soc/amd/stoneyridge/include/soc/i2c.h @@ -0,0 +1,49 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2018 Advanced Micro Devices, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef __STONEYRIDGE_I2C_H__ +#define __STONEYRIDGE_I2C_H__ + +#include <soc/gpio.h> + +struct soc_amd_i2c_save { + uint32_t control_value; + uint8_t mux_value; +}; + +#define GPIO_I2C0_SCL BIT(0) +#define GPIO_I2C1_SCL BIT(1) +#define GPIO_I2C2_SCL BIT(2) +#define GPIO_I2C3_SCL BIT(3) +#define GPIO_I2C_MASK (BIT(0) | BIT(1) | BIT(2) | BIT(3)) + +#define I2C0_SCL_PIN GPIO_145 +#define I2C1_SCL_PIN GPIO_147 +#define I2C2_SCL_PIN GPIO_113 +#define I2C3_SCL_PIN GPIO_19 + +#define GPIO_I2C0_ADDRESS GPIO_BANK2_CONTROL(I2C0_SCL_PIN) +#define GPIO_I2C1_ADDRESS GPIO_BANK2_CONTROL(I2C1_SCL_PIN) +#define GPIO_I2C2_ADDRESS GPIO_BANK1_CONTROL(I2C2_SCL_PIN) +#define GPIO_I2C3_ADDRESS GPIO_BANK0_CONTROL(I2C3_SCL_PIN) + +#define I2C0_SCL_PIN_IOMUX_GPIOxx GPIO_145_IOMUX_GPIOxx +#define I2C1_SCL_PIN_IOMUX_GPIOxx GPIO_147_IOMUX_GPIOxx +#define I2C2_SCL_PIN_IOMUX_GPIOxx GPIO_113_IOMUX_GPIOxx +#define I2C3_SCL_PIN_IOMUX_GPIOxx GPIO_19_IOMUX_GPIOxx + +void sb_reset_i2c_slaves(void); + +#endif /* __STONEYRIDGE_I2C_H__ */ -- To view, visit https://review.coreboot.org/c/coreboot/+/32650 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3b2d8e1b54e7c5929220d763bd99fe01b0636aaa Gerrit-Change-Number: 32650 Gerrit-PatchSet: 1 Gerrit-Owner: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Richard Spiegel <richard.spiegel(a)silverbackltd.com> Gerrit-MessageType: newchange

4 4

Change in ...coreboot[master]: soc/intel/skylake: Correct GPIO pointer assignment
by Jacob Garber (Code Review) 16 May '19

16 May '19

Jacob Garber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32797 Change subject: soc/intel/skylake: Correct GPIO pointer assignment ...................................................................... soc/intel/skylake: Correct GPIO pointer assignment We need to store the acpi_gpio struct, not save its address. Found-by: Clang Static Analyzer Signed-off-by: Jacob Garber <jgarber1(a)ualberta.ca> Change-Id: I41c8bf10ce72bec736da97ccc33f9ada49804dc1 --- M src/soc/intel/skylake/sd.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/97/32797/1 diff --git a/src/soc/intel/skylake/sd.c b/src/soc/intel/skylake/sd.c index 12c6abe..571d3e7 100644 --- a/src/soc/intel/skylake/sd.c +++ b/src/soc/intel/skylake/sd.c @@ -35,7 +35,7 @@ gpio->pin_count = 1; gpio->pins[0] = config->sdcard_cd_gpio_default; } else - gpio = &config->sdcard_cd_gpio; + *gpio = config->sdcard_cd_gpio; return 0; } -- To view, visit https://review.coreboot.org/c/coreboot/+/32797 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I41c8bf10ce72bec736da97ccc33f9ada49804dc1 Gerrit-Change-Number: 32797 Gerrit-PatchSet: 1 Gerrit-Owner: Jacob Garber <jgarber1(a)ualberta.ca> Gerrit-MessageType: newchange

3 2

Change in ...coreboot[master]: superio: Add ASpeed AST2400
by Patrick Rudolph (Code Review) 16 May '19

16 May '19

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/23135 ) Change subject: superio: Add ASpeed AST2400 ...................................................................... Patch Set 23: (2 comments) Tested on a supermicro board, works fine. https://review.coreboot.org/#/c/23135/17/src/superio/aspeed/Makefile.inc File src/superio/aspeed/Makefile.inc: https://review.coreboot.org/#/c/23135/17/src/superio/aspeed/Makefile.inc@18 PS17, Line 18: romstage-$(CONFIG_SUPERIO_ASPEED_COMMON_ROMSTAGE) += common/early_serial.c > Done not done in latest revision. https://review.coreboot.org/#/c/23135/17/src/superio/aspeed/common/Kconfig File src/superio/aspeed/common/Kconfig: https://review.coreboot.org/#/c/23135/17/src/superio/aspeed/common/Kconfig@… PS17, Line 20: ROMSTAGE > Done not done in latest revision. -- To view, visit https://review.coreboot.org/c/coreboot/+/23135 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I58fce31f0a2483e61e9d31f38ab5a059b8cf4f83 Gerrit-Change-Number: 23135 Gerrit-PatchSet: 23 Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-CC: Patrick Rudolph <siro(a)das-labor.org> Gerrit-CC: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Comment-Date: Thu, 16 May 2019 09:38:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: mainboard/facebook/fbg1701: Do initial mainboard commit
by Frans Hendriks (Code Review) 16 May '19

16 May '19

Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30414 ) Change subject: mainboard/facebook/fbg1701: Do initial mainboard commit ...................................................................... Patch Set 22: (3 comments) Will upload a new patch set https://review.coreboot.org/#/c/30414/22/src/mainboard/facebook/fbg1701/acp… File src/mainboard/facebook/fbg1701/acpi/ec.asl: https://review.coreboot.org/#/c/30414/22/src/mainboard/facebook/fbg1701/acp… PS22, Line 1: /* > Can this file be removed? No, this file required. (Included by src/soc/intel/braswell/acpi/lpc.asl) https://review.coreboot.org/#/c/30414/22/src/mainboard/facebook/fbg1701/log… File src/mainboard/facebook/fbg1701/logo.c: https://review.coreboot.org/#/c/30414/22/src/mainboard/facebook/fbg1701/log… PS22, Line 35: size_t file_size = cbfs_boot_load_file(filename, logo_data, > As Michal suggested in PS17, you can get rid of logo_array[] and use the return value from cbfs_boot […] cbfs_boot_map_with_leak() does not work on compressed logo. Need to uncompress the image. (The used code is similar to vbt.bin handling) https://review.coreboot.org/#/c/30414/22/src/mainboard/facebook/fbg1701/smi… File src/mainboard/facebook/fbg1701/smihandler.c: https://review.coreboot.org/#/c/30414/22/src/mainboard/facebook/fbg1701/smi… PS22, Line 22: int mainboard_io_trap_handler(int smif) > Is any of this needed? If we just need the function definition to be present, would it be better to […] Will remove this file. Weak function will be used return 0. -- To view, visit https://review.coreboot.org/c/coreboot/+/30414 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I28ac78a630ee705b1e546031f024bfe7f952ab39 Gerrit-Change-Number: 30414 Gerrit-PatchSet: 22 Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: David Hendricks <david.hendricks(a)gmail.com> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Comment-Date: Thu, 16 May 2019 07:53:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: David Hendricks <david.hendricks(a)gmail.com> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2019