Martin Roth has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32407
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
soc/amd/picasso: Create picasso as a copy of stoneyridge
So that everyone can see what's being updated from stoney, we're starting with a direct copy of the stoney directory. There are arguments both for and against doing it this way, but I believe it's the most transparent
Makefile.inc has been renamed as makefile.inc until it's updated.
TEST=None BUG=b:130804851
Signed-off-by: Martin Roth martinroth@chromium.org Change-Id: I6809bd1eea304f76dd9000c079b3ed09f94dbd3b --- A src/soc/amd/picasso/BiosCallOuts.c A src/soc/amd/picasso/Kconfig A src/soc/amd/picasso/acpi.c A src/soc/amd/picasso/acpi/acpi_wake_source.asl A src/soc/amd/picasso/acpi/cpu.asl A src/soc/amd/picasso/acpi/globalnvs.asl A src/soc/amd/picasso/acpi/gpio_lib.asl A src/soc/amd/picasso/acpi/lpc.asl A src/soc/amd/picasso/acpi/northbridge.asl A src/soc/amd/picasso/acpi/pci_int.asl A src/soc/amd/picasso/acpi/pcie.asl A src/soc/amd/picasso/acpi/sb_fch.asl A src/soc/amd/picasso/acpi/sb_pci0_fch.asl A src/soc/amd/picasso/acpi/sleepstates.asl A src/soc/amd/picasso/acpi/soc.asl A src/soc/amd/picasso/acpi/usb.asl A src/soc/amd/picasso/bootblock/bootblock.c A src/soc/amd/picasso/chip.c A src/soc/amd/picasso/chip.h A src/soc/amd/picasso/cpu.c A src/soc/amd/picasso/enable_usbdebug.c A src/soc/amd/picasso/finalize.c A src/soc/amd/picasso/gpio.c A src/soc/amd/picasso/hda.c A src/soc/amd/picasso/i2c.c A src/soc/amd/picasso/include/soc/acpi.h A src/soc/amd/picasso/include/soc/amd_pci_int_defs.h A src/soc/amd/picasso/include/soc/cpu.h A src/soc/amd/picasso/include/soc/gpio.h A src/soc/amd/picasso/include/soc/iomap.h A src/soc/amd/picasso/include/soc/northbridge.h A src/soc/amd/picasso/include/soc/nvs.h A src/soc/amd/picasso/include/soc/pci_devs.h A src/soc/amd/picasso/include/soc/romstage.h A src/soc/amd/picasso/include/soc/smbus.h A src/soc/amd/picasso/include/soc/smi.h A src/soc/amd/picasso/include/soc/southbridge.h A src/soc/amd/picasso/iommu.c A src/soc/amd/picasso/lpc.c A src/soc/amd/picasso/makefile.inc A src/soc/amd/picasso/mca.c A src/soc/amd/picasso/monotonic_timer.c A src/soc/amd/picasso/nb_util.c A src/soc/amd/picasso/northbridge.c A src/soc/amd/picasso/pmutil.c A src/soc/amd/picasso/ramtop.c A src/soc/amd/picasso/reset.c A src/soc/amd/picasso/romstage.c A src/soc/amd/picasso/sata.c A src/soc/amd/picasso/sb_util.c A src/soc/amd/picasso/sm.c A src/soc/amd/picasso/smbus.c A src/soc/amd/picasso/smbus_spd.c A src/soc/amd/picasso/smi.c A src/soc/amd/picasso/smi_util.c A src/soc/amd/picasso/smihandler.c A src/soc/amd/picasso/southbridge.c A src/soc/amd/picasso/spi.c A src/soc/amd/picasso/tsc_freq.c A src/soc/amd/picasso/uart.c A src/soc/amd/picasso/usb.c 61 files changed, 11,242 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/32407/1
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 1:
(11 comments)
https://review.coreboot.org/#/c/32407/1/src/soc/amd/picasso/bootblock/bootbl... File src/soc/amd/picasso/bootblock/bootblock.c:
https://review.coreboot.org/#/c/32407/1/src/soc/amd/picasso/bootblock/bootbl... PS1, Line 83: void (*ap_romstage_entry)(void) = function definition argument 'void' should also have an identifier name
https://review.coreboot.org/#/c/32407/1/src/soc/amd/picasso/include/soc/sout... File src/soc/amd/picasso/include/soc/southbridge.h:
https://review.coreboot.org/#/c/32407/1/src/soc/amd/picasso/include/soc/sout... PS1, Line 384: #define SPI_READ_MODE_DUAL112 ( BIT(29) ) space prohibited after that open parenthesis '('
https://review.coreboot.org/#/c/32407/1/src/soc/amd/picasso/include/soc/sout... PS1, Line 384: #define SPI_READ_MODE_DUAL112 ( BIT(29) ) space prohibited before that close parenthesis ')'
https://review.coreboot.org/#/c/32407/1/src/soc/amd/picasso/include/soc/sout... PS1, Line 385: #define SPI_READ_MODE_QUAD114 ( BIT(29) | BIT(18)) space prohibited after that open parenthesis '('
https://review.coreboot.org/#/c/32407/1/src/soc/amd/picasso/include/soc/sout... PS1, Line 386: #define SPI_READ_MODE_DUAL122 (BIT(30) ) space prohibited before that close parenthesis ')'
https://review.coreboot.org/#/c/32407/1/src/soc/amd/picasso/include/soc/sout... PS1, Line 388: #define SPI_READ_MODE_NORMAL66 (BIT(30) | BIT(29) ) space prohibited before that close parenthesis ')'
https://review.coreboot.org/#/c/32407/1/src/soc/amd/picasso/include/soc/sout... PS1, Line 415: #define SPI_SPEED_33M ( BIT(0)) space prohibited after that open parenthesis '('
https://review.coreboot.org/#/c/32407/1/src/soc/amd/picasso/include/soc/sout... PS1, Line 416: #define SPI_SPEED_22M ( BIT(1) ) space prohibited after that open parenthesis '('
https://review.coreboot.org/#/c/32407/1/src/soc/amd/picasso/include/soc/sout... PS1, Line 416: #define SPI_SPEED_22M ( BIT(1) ) space prohibited before that close parenthesis ')'
https://review.coreboot.org/#/c/32407/1/src/soc/amd/picasso/include/soc/sout... PS1, Line 417: #define SPI_SPEED_16M ( BIT(1) | BIT(0)) space prohibited after that open parenthesis '('
https://review.coreboot.org/#/c/32407/1/src/soc/amd/picasso/include/soc/sout... PS1, Line 418: #define SPI_SPEED_100M (BIT(2) ) space prohibited before that close parenthesis ')'
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 1: Code-Review-1
(2 comments)
Copy first (let sb else) care later defeats review, destroys future Git workflow and slows the project down.
https://review.coreboot.org/#/c/32407/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32407/1//COMMIT_MSG@9 PS1, Line 9: everyone can see what's being updated from stoney What's the benefit in that?
https://review.coreboot.org/#/c/32407/1//COMMIT_MSG@12 PS1, Line 12: it's the most transparent Please remind me, what are the arguments for it? So far I thought it's just "this is how we do it since the '80s".
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 1: Code-Review+1
(2 comments)
https://review.coreboot.org/#/c/32407/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32407/1//COMMIT_MSG@12 PS1, Line 12: it's the most transparent
Please remind me, what are the arguments for it? So far I thought […]
The topic should be discussed on the mailing list.
The biggest argument is, that the present code has been reviewed, and does not have to be reviewed again.
https://review.coreboot.org/#/c/32407/1//COMMIT_MSG@15 PS1, Line 15: Martin, maybe you can mention explicitly, that both devices are pretty similar in several aspects, so copying makes sense.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/#/c/32407/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32407/1//COMMIT_MSG@12 PS1, Line 12: it's the most transparent
The biggest argument is, that the present code has been reviewed,
This is true.
and does not have to be reviewed again.
This is very subjective. As somebody who takes what the code does into account and not only how it looks like, I state the exact opposite: It has to be reviewed again (for the new silicon).
I have seen it very often now that after the first train of copy- first patches, the development seems to turn into a bug fixing because nobody keeps track any more which part of the code has already been reviewed/adapted for the new silicon. So if something doesn't work, I assume a developer will start digging through the code and look for things that might have to be adapted. Is that documented somewhere that he did so? I doubt it. So the next one with an issue has to go through everything again, just because the information is lost what parts are already re-reviewed. In the long run all this information is completely lost, even though we actually chose Git to preserve such knowledge.
That's just the Git-history view on the matter. I could go on about how the code bases tend to deviate and what that means for maintenance...
https://review.coreboot.org/#/c/32407/1/src/soc/amd/picasso/smbus.c File src/soc/amd/picasso/smbus.c:
PS1: Nice code, wow you added 237 lines to coreboot, thanks! Maybe you missed that we already have half a dozen copies of it?
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/32407/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32407/1//COMMIT_MSG@15 PS1, Line 15:
Martin, maybe you can mention explicitly, that both devices are pretty similar in several aspects, s […]
Btw. if the hardware were the same, copying would make less sense, right? So the more similar they are the less copying+adapting makes sense and not the other way around.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 1: Code-Review-1
IMHO, if the SoCs are similar, copy-pasting a bunch of code which results in two similar codebases doesn't make sense, and probably results in some extra maintenance burden. I'd rather see common code instead (to the extent that is possible).
IMHO, if the SoCs differ significantly, copy-pasting code then changing it sounds completely insane. I doubt that this is the case here, but in case it is, I believe the most logical approach would be "starting fresh", i.e., adding properly updated files from the start, in a patch train.
NB I am not an expert and may be mistaken.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 1:
Patch Set 1: Code-Review-1
IMHO, if the SoCs are similar, copy-pasting a bunch of code which results in two similar codebases doesn't make sense, and probably results in some extra maintenance burden. I'd rather see common code instead (to the extent that is possible).
Sure, and we can refactor it at some point. I'm not sure that doing that while we're actively writing the code and trying figure out what we can keep and what needs to be changed. We can definitely create bugs to go back and refactor things.
IMHO, if the SoCs differ significantly, copy-pasting code then changing it sounds completely insane. I doubt that this is the case here, but in case it is, I believe the most logical approach would be "starting fresh", i.e., adding properly updated files from the start, in a patch train.
Some things are similar and some things are different. The approach that was taken for development was to base the new SOC on the old SOC code
NB I am not an expert and may be mistaken.
As I said, there points to be made for both ways. Honestly, this is the more difficult approach because I have to break things down to incremental changes. That makes it easier for people to review than just pushing the new files with all of the changes already in place, but if that's what people prefer, I'd be glad to do that.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 1:
Quite honestly, I feel like people trying to develop new platforms can't win, and I'm a long-time developer on the project. I absolutely cannot imagine coming into the community as a new person and trying to add something.
I'm trying to do the right thing here and make it easy for the coreboot community to see what's being changed. I'm trying to push early instead of doing all of the development in a closed codebase, making all the decision on my own, and then trying to push it.
From Paul's comment, I get the feeling that because the Zen processor is so different, it might not be accepted in the coreboot codebase. Why else do we need to talk about it on the mailing list.
I go to extra effort to break everything down into small pieces that can be easily reviewed, and I feel attacked for that.
We need to pick a method for accepting new chips and go with it. If you want whole files that are finished, that's fine with me. If you want to force the development to be done elsewhere, I'm fine with that too.
While I do work at Google, I'm a part of the coreboot community. I'm working at Google BECAUSE i'm a part of the community. I understand the issues. I don't like the blobs and lack of documentation either. Although it may still be going in the wrong direction I'm actively working to try to improve things.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 1:
As I said, there points to be made for both ways. Honestly, this is the more difficult approach because I have to break things down to incremental changes. That makes it easier for people to review than just pushing the new files with all of the changes already in place, but if that's what people prefer, I'd be glad to do that.
Your approach makes it easier to review changed lines, yes. But it makes it nearly impossible to review what isn't changed. I know some reviewers prefer that, simply because it means less work for them. One can rubber stamp the first commit and blame the process if problems sneak in.
Please note, I haven't seen any datasheet from AMD yet, so I'll probably never be able to +2 any of this. I just care about it because I often try to maintain things across the tree and would probably give up if the only commit message I can find says nothing but "transparency, yeah!". It's very hard to work with the copy-pasta code base that our AMD stuff is, you can never tell by the Git history if a piece of code actually works on the platform you are looking at (or was even meant to be ever run on it).
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 1: -Code-Review
Patch Set 1:
Quite honestly, I feel like people trying to develop new platforms can't win, and I'm a long-time developer on the project. I absolutely cannot imagine coming into the community as a new person and trying to add something.
I'm trying to do the right thing here and make it easy for the coreboot community to see what's being changed. I'm trying to push early instead of doing all of the development in a closed codebase, making all the decision on my own, and then trying to push it.
From Paul's comment, I get the feeling that because the Zen processor is so different, it might not be accepted in the coreboot codebase. Why else do we need to talk about it on the mailing list.
I go to extra effort to break everything down into small pieces that can be easily reviewed, and I feel attacked for that.
We need to pick a method for accepting new chips and go with it. If you want whole files that are finished, that's fine with me. If you want to force the development to be done elsewhere, I'm fine with that too.
While I do work at Google, I'm a part of the coreboot community. I'm working at Google BECAUSE i'm a part of the community. I understand the issues. I don't like the blobs and lack of documentation either. Although it may still be going in the wrong direction I'm actively working to try to improve things.
Not sure if this was meant for me, but don't get me wrong: I would be glad to see new platforms merged in, especially on the AMD front (I don't do much non-x86). What I would not want to see is code that ends up bitrotting because nobody understands it.
I've removed my vote; I have absolutely no say on this matter.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 1:
Well the stoneyridge source base you copy from is far from perfect, and seems to have reached the same "go runaway" approach as it was in the days of SAGE, as Google / Silverback collaboration with AMD seems to now refuse to do any reviews on it.
https://review.coreboot.org/q/topic:%22stoney"
Also back in 2017 conference I was told by Google staff that this StoneyPI source would be scrubbed and relicensed once related Chromebook hardware ships. Any news on the scrubbing?
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 1:
Patch Set 1: -Code-Review
Patch Set 1:
Quite honestly, I feel like people trying to develop new platforms can't win, and I'm a long-time developer on the project. I absolutely cannot imagine coming into the community as a new person and trying to add something.
I'm trying to do the right thing here and make it easy for the coreboot community to see what's being changed. I'm trying to push early instead of doing all of the development in a closed codebase, making all the decision on my own, and then trying to push it.
From Paul's comment, I get the feeling that because the Zen processor is so different, it might not be accepted in the coreboot codebase. Why else do we need to talk about it on the mailing list.
I go to extra effort to break everything down into small pieces that can be easily reviewed, and I feel attacked for that.
We need to pick a method for accepting new chips and go with it. If you want whole files that are finished, that's fine with me. If you want to force the development to be done elsewhere, I'm fine with that too.
While I do work at Google, I'm a part of the coreboot community. I'm working at Google BECAUSE i'm a part of the community. I understand the issues. I don't like the blobs and lack of documentation either. Although it may still be going in the wrong direction I'm actively working to try to improve things.
Not sure if this was meant for me, but don't get me wrong: I would be glad to see new platforms merged in, especially on the AMD front (I don't do much non-x86). What I would not want to see is code that ends up bitrotting because nobody understands it.
I've removed my vote; I have absolutely no say on this matter.
Hi Angel, Sorry if this felt like I was attacking you, that was not my intention. I'm obviously frustrated. I totally understand that people may not agree with what I'm pushing or how I chose to push it, but I'm *TRYING* to do the right thing.
My frustration is because if I chose the other path, of pushing fully finished pieces, I'd get objections for doing it that way as well. It really seems like there's no good way to get new platforms in. I started the patch by making that comment, and I get sarcastic comments. Again, Angel, I'm not referring to your review.
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/32407/1/src/soc/amd/picasso/acpi.c File src/soc/amd/picasso/acpi.c:
https://review.coreboot.org/#/c/32407/1/src/soc/amd/picasso/acpi.c@72 PS1, Line 72: void acpi_create_fadt(acpi_fadt_t *fadt, acpi_facs_t *facs, void *dsdt) sorry, maybe my question is out of topic. but this function is defined in included <arch/acpi.h> and we can find also the same function name here: src/arch/x86/acpi.c .
so the compiler will use current function or the one from "src/arch/x86/acpi.c"?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 1:
Patch Set 1: Hi Angel, Sorry if this felt like I was attacking you, that was not my intention. I'm obviously frustrated. I totally understand that people may not agree with what I'm pushing or how I chose to push it, but I'm *TRYING* to do the right thing.
My frustration is because if I chose the other path, of pushing fully finished pieces, I'd get objections for doing it that way as well. It really seems like there's no good way to get new platforms in. I started the patch by making that comment, and I get sarcastic comments. Again, Angel, I'm not referring to your review.
I understand, thank you for clarifying :)
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 1:
Martin, I feel your pain here. But is this platform AGESA v8 or AGESA v9?
You should revisit old reviews when StoneyPI commits first started to appear. Go back to those early 2017 (?) days where I have to fight against Marc and Marshall about somewhat "simple things" like countless of TODOs and FIXMEs that were left behind and copy-pasted from previous binaryPI, again copied from AGESA. At times, there were (accidental) pushes that just removed some TODO comments withoutexplanations.
One thing came clear during StoneyPI reviews; even for staff at Silverback AGESA v8 and agesawrapper.c in particular was a complete mis(t)ery. Revisit reviews, and notice how many times I personally had to step in on the reviews and tell the code you had copied from binaryPI was not good at all.
https://blogs.coreboot.org/blog/2014/08/03/gsoc-infrastructure-along-the-way...
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 1:
Patch Set 1:
Well the stoneyridge source base you copy from is far from perfect, and seems to have reached the same "go runaway" approach as it was in the days of SAGE, as Google / Silverback collaboration with AMD seems to now refuse to do any reviews on it.
https://review.coreboot.org/q/topic:%22stoney"
Also back in 2017 conference I was told by Google staff that this StoneyPI source would be scrubbed and relicensed once related Chromebook hardware ships. Any news on the scrubbing?
AMD has no engineers to do any coreboot work. They are outsourcing to Silverback. It's not that they refuse to review anything, but they have nobody to do it.
Silverback was supposed to go back and fix any issues with the Stoney code. I was under the impression that that was already done, as all outstanding bugs were fixed. If it's not, please help identify areas that need improvement and I'll work to get things changed. This is especially important as it IS being used as a base for the Picasso work.
On the issue of releasing the StoneyPI code, AMD did agree to release it, but when they started working towards that, found that because of internal issues, they had no way to do that. I don't even know what to say beyond that. :( I'm sorry, I didn't get it written into a contract and I have no way to force the issue.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 1:
Martin, I feel your pain here. But is this platform AGESA v8 or AGESA v9?
This is AGESA V9. It's going to need a completely different interface.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 1:
Silverback was supposed to go back and fix any issues with the Stoney code. I was under the impression that that was already done, as all outstanding bugs were fixed. If it's not, please help identify areas that need improvement and I'll work to get things changed. This is especially important as it IS being used as a base for the Picasso work.
With original AGESA work S3 suspend support was submitted as "working". Just that it always wrote 32 KiB of SPI flash on every boot. And corrupted low memory on resume. It was neither AMD or Silverback that fixed those, but me.
'Reported bugs fixed' does not equal 'Good code that needs no review'.
I don't have bugfixes to report, just an implementation of the AGESA API tha was completely incomprehensible to the staff to deal with and here are more recent proofs of that:
https://review.coreboot.org/c/coreboot/+/27112 https://review.coreboot.org/c/coreboot/+/27277
On the issue of releasing the StoneyPI code, AMD did agree to release it, but when they started
working towards that, found that because of internal issues, they had no way to do that. I don't even know what to say beyond that. :( I'm sorry, I didn't get it written into a contract and I have no way to force the issue.
Yeah.. decisions like that will likely keep me away from any picasso reviews. Take it as a blessing or a loss.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 1:
On the issue of releasing the StoneyPI code, AMD did agree to release it, but when they started
working towards that, found that because of internal issues, they had no way to do that. I don't even know what to say beyond that. :( I'm sorry, I didn't get it written into a contract and I have no way to force the issue.
Yeah.. decisions like that will likely keep me away from any picasso reviews. Take it as a blessing or a loss.
I don't blame you. And again, I'm sorry. Believe me, I'm incredibly frustrated as well. I think you know that I appreciate your reviews and work. What you've done for the coreboot project is invaluable.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 1:
(1 comment)
From Paul's comment, I get the feeling that because the Zen processor is so different, it might not be accepted in the coreboot codebase. Why else do we need to talk about it on the mailing list.
I think you got caught in the crossfire here. He replied to my comment, so I assume he meant that I (or we) should take the commit-style discussion to the mailing list. Actually, to separate it from your commits.
I go to extra effort to break everything down into small pieces that can be easily reviewed, and I feel attacked for that.
Your effort is much appreciated. No matter how much I lament about the copy-first, it would be obviously worse if you'd push only two commits copy+adapt.
We need to pick a method for accepting new chips and go with it. If you want whole files that are finished, that's fine with me. If you want to force the development to be done elsewhere, I'm fine with that too.
I'm not sure if I understand this correctly. You seem to say that pushing whole files implies that you have to first finish the whole development and then break it apart again? Is that it?
I guess that might be true for a small, core part of the code, but most likely not for the whole 11k LOC. Maybe I'm wrong here but the code looks too well organized to be hard to break apart, or rather to pick one topic after another from it.
While I do work at Google, I'm a part of the coreboot community. I'm working at Google BECAUSE i'm a part of the community. I understand the issues. I don't like the blobs and lack of documentation either. Although it may still be going in the wrong direction I'm actively working to try to improve things.
Um, I'm confused that you bring your employment and blobs into this. Did sb. (I?) bring that up somewhere and forgot? Or is there an argument? If you wouldn't be a part of the community (which is great that you are btw.) and working for Google, this discussion would be less unexpected? I'm really puzzled here.
https://review.coreboot.org/#/c/32407/1/src/soc/amd/picasso/acpi.c File src/soc/amd/picasso/acpi.c:
https://review.coreboot.org/#/c/32407/1/src/soc/amd/picasso/acpi.c@72 PS1, Line 72: void acpi_create_fadt(acpi_fadt_t *fadt, acpi_facs_t *facs, void *dsdt)
sorry, maybe my question is out of topic. […]
The version in arch/x86/ is guarded by an #if.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 1:
We need to pick a method for accepting new chips and go with it. If you want whole files that are finished, that's fine with me. If you want to force the development to be done elsewhere, I'm fine with that too.
I'm not sure if I understand this correctly. You seem to say that pushing whole files implies that you have to first finish the whole development and then break it apart again? Is that it?
I guess that might be true for a small, core part of the code, but most likely not for the whole 11k LOC. Maybe I'm wrong here but the code looks too well organized to be hard to break apart, or rather to pick one topic after another from it.
We've already been working on the project for a bit while we've been waiting for permission to start upstreaming it. Because of that we've got a backlog of changes to push. It's not a large backlog at this point, but it's not insignificant. That work's just been done on individual machines so far - I've got to go through that code and break it into small cohesive bits.
So I could just push the changed files, which include changes from various different topics, but that would make it hard to review.
While I do work at Google, I'm a part of the coreboot community. I'm working at Google BECAUSE i'm a part of the community. I understand the issues. I don't like the blobs and lack of documentation either. Although it may still be going in the wrong direction I'm actively working to try to improve things.
Um, I'm confused that you bring your employment and blobs into this. Did sb. (I?) bring that up somewhere and forgot? Or is there an argument? If you wouldn't be a part of the community (which is great that you are btw.) and working for Google, this discussion would be less unexpected? I'm really puzzled here.
No, you didn't say anything, but you can't deny that there's a lot of resentment about large companies (such as Google) in some sectors of the community. I'm trying to say (Not specifically to you) that I'm working as a part of the community, not just as a representative for Google. I feel the same frustration with the blob situation, and I'm sorry. My opinion is that it's best to work with the companies to show why it's better to publish. Other people feel like it's better to boycott and badmouth the companies. (Nico, I'm not pointing at you in saying this. We've all seen the discussions on IRC.) I get it - people feel strongly. We've both put countless hours into the project, far more than most people who want to sit on the sidelines and criticize.
If I were entering the community for the first time, trying to commit a new SOC, it would look insurmountable. I'm saying that we should have a defined method of how to push and maintain these things that we can agree upon as a community. - Do we start with an existing codebase so that the changes can easily be seen? There are people in the community who feel strongly about both methods. - If a file already exists and you're pushing it, how does it need to be broken up for review? If so, how? - What's the requirement for going back and refactoring old code when you're adding new code? Can it be copied initially and then refactored as the project progresses, or do you have to refactor it into a common location before you'll be allowed to push in your chip? - What's required in terms of long-term-maintenance when a new SOC is pushed? Does the requirement to maintain it end when the project ends, or is there an implied agreement to continue maintaining it? If so, how long? - What's expected when someone points out a problem in the code? - What sort of documentation is required on the SOC? - Is the expectation different if an individual is trying to add a new chip than if a company is pushing it. If so, why?
Marshall Dawson has uploaded a new patch set (#2) to the change originally created by Martin Roth. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
soc/amd/picasso: Create picasso as a copy of stoneyridge
So that everyone can see what's being updated from stoney, we're starting with a direct copy of the stoney directory. There are arguments both for and against doing it this way, but I believe it's the most transparent.
Makefile.inc has been renamed as makefile.inc until it's updated.
TEST=None BUG=b:130804851
Signed-off-by: Martin Roth martinroth@chromium.org Change-Id: I6809bd1eea304f76dd9000c079b3ed09f94dbd3b --- A src/soc/amd/picasso/BiosCallOuts.c A src/soc/amd/picasso/Kconfig A src/soc/amd/picasso/acpi.c A src/soc/amd/picasso/acpi/acpi_wake_source.asl A src/soc/amd/picasso/acpi/cpu.asl A src/soc/amd/picasso/acpi/globalnvs.asl A src/soc/amd/picasso/acpi/northbridge.asl A src/soc/amd/picasso/acpi/pci_int.asl A src/soc/amd/picasso/acpi/pcie.asl A src/soc/amd/picasso/acpi/sb_fch.asl A src/soc/amd/picasso/acpi/sb_pci0_fch.asl A src/soc/amd/picasso/acpi/sleepstates.asl A src/soc/amd/picasso/acpi/soc.asl A src/soc/amd/picasso/acpi/usb.asl A src/soc/amd/picasso/bootblock/bootblock.c A src/soc/amd/picasso/chip.c A src/soc/amd/picasso/chip.h A src/soc/amd/picasso/cpu.c A src/soc/amd/picasso/enable_usbdebug.c A src/soc/amd/picasso/finalize.c A src/soc/amd/picasso/gpio.c A src/soc/amd/picasso/i2c.c A src/soc/amd/picasso/include/soc/acpi.h A src/soc/amd/picasso/include/soc/amd_pci_int_defs.h A src/soc/amd/picasso/include/soc/cpu.h A src/soc/amd/picasso/include/soc/gpio.h A src/soc/amd/picasso/include/soc/i2c.h A src/soc/amd/picasso/include/soc/iomap.h A src/soc/amd/picasso/include/soc/northbridge.h A src/soc/amd/picasso/include/soc/nvs.h A src/soc/amd/picasso/include/soc/pci_devs.h A src/soc/amd/picasso/include/soc/romstage.h A src/soc/amd/picasso/include/soc/smbus.h A src/soc/amd/picasso/include/soc/smi.h A src/soc/amd/picasso/include/soc/southbridge.h A src/soc/amd/picasso/makefile.inc A src/soc/amd/picasso/mca.c A src/soc/amd/picasso/monotonic_timer.c A src/soc/amd/picasso/nb_util.c A src/soc/amd/picasso/northbridge.c A src/soc/amd/picasso/pmutil.c A src/soc/amd/picasso/ramtop.c A src/soc/amd/picasso/reset.c A src/soc/amd/picasso/romstage.c A src/soc/amd/picasso/sata.c A src/soc/amd/picasso/sm.c A src/soc/amd/picasso/smbus.c A src/soc/amd/picasso/smbus_spd.c A src/soc/amd/picasso/smi.c A src/soc/amd/picasso/smi_util.c A src/soc/amd/picasso/smihandler.c A src/soc/amd/picasso/southbridge.c A src/soc/amd/picasso/spi.c A src/soc/amd/picasso/tsc_freq.c A src/soc/amd/picasso/uart.c A src/soc/amd/picasso/usb.c 56 files changed, 9,198 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/32407/2
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 2:
(11 comments)
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/bootblock/bootbl... File src/soc/amd/picasso/bootblock/bootblock.c:
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/bootblock/bootbl... PS2, Line 83: void (*ap_romstage_entry)(void) = function definition argument 'void' should also have an identifier name
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/sout... File src/soc/amd/picasso/include/soc/southbridge.h:
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/sout... PS2, Line 265: #define SPI_READ_MODE_DUAL112 ( BIT(29) ) space prohibited after that open parenthesis '('
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/sout... PS2, Line 265: #define SPI_READ_MODE_DUAL112 ( BIT(29) ) space prohibited before that close parenthesis ')'
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/sout... PS2, Line 266: #define SPI_READ_MODE_QUAD114 ( BIT(29) | BIT(18)) space prohibited after that open parenthesis '('
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/sout... PS2, Line 267: #define SPI_READ_MODE_DUAL122 (BIT(30) ) space prohibited before that close parenthesis ')'
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/sout... PS2, Line 269: #define SPI_READ_MODE_NORMAL66 (BIT(30) | BIT(29) ) space prohibited before that close parenthesis ')'
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/sout... PS2, Line 296: #define SPI_SPEED_33M ( BIT(0)) space prohibited after that open parenthesis '('
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/sout... PS2, Line 297: #define SPI_SPEED_22M ( BIT(1) ) space prohibited after that open parenthesis '('
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/sout... PS2, Line 297: #define SPI_SPEED_22M ( BIT(1) ) space prohibited before that close parenthesis ')'
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/sout... PS2, Line 298: #define SPI_SPEED_16M ( BIT(1) | BIT(0)) space prohibited after that open parenthesis '('
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/sout... PS2, Line 299: #define SPI_SPEED_100M (BIT(2) ) space prohibited before that close parenthesis ')'
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 2:
Nico, Marshall has done a bunch of cleanup on stoney. Is there more that you feel needs to be done before making this copy?
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 2:
(108 comments)
Way too many stoney or stoneyridge. I hope I found them all.
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/BiosCallOuts.c File src/soc/amd/picasso/BiosCallOuts.c:
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/BiosCallOuts.c@6... PS2, Line 63: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/BiosCallOuts.c@7... PS2, Line 70: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/BiosCallOuts.c@7... PS2, Line 72: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/BiosCallOuts.c@1... PS2, Line 102: stoneyridge picasso
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/Kconfig File src/soc/amd/picasso/Kconfig:
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/Kconfig@16 PS2, Line 16: config SOC_AMD_STONEYRIDGE_FP4 : bool : help : AMD Stoney Ridge FP4 support : : config SOC_AMD_STONEYRIDGE_FT4 : bool : help : AMD Stoney Ridge FT4 support : : if SOC_AMD_STONEYRIDGE_FP4 || SOC_AMD_STONEYRIDGE_FT4 : PICASSO, how many sockets will it support?
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/Kconfig@145 PS2, Line 145: stoneyridge picasso
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/Kconfig@159 PS2, Line 159: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/Kconfig@160 PS2, Line 160: Stoney Ridge Picasso
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/Kconfig@169 PS2, Line 169: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/Kconfig@173 PS2, Line 173: Stoney Ridge Picasso
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/Kconfig@175 PS2, Line 175: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/Kconfig@179 PS2, Line 179: Stoney Ridge Picasso
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/Kconfig@182 PS2, Line 182: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/Kconfig@184 PS2, Line 184: stoneyridge picasso
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/Kconfig@185 PS2, Line 185: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/Kconfig@187 PS2, Line 187: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/Kconfig@189 PS2, Line 189: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/Kconfig@193 PS2, Line 193: stoneyridge picasso
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/Kconfig@195 PS2, Line 195: TONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/Kconfig@211 PS2, Line 211: TONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/Kconfig@214 PS2, Line 214: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/Kconfig@217 PS2, Line 217: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/Kconfig@220 PS2, Line 220: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/Kconfig@223 PS2, Line 223: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/Kconfig@226 PS2, Line 226: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/Kconfig@228 PS2, Line 228: if STONEYRIDGE_SATA_MODE = 2 || STONEYRIDGE_SATA_MODE = 5 PICASSO, 2 places
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/Kconfig@232 PS2, Line 232: default "1022,7801" if STONEYRIDGE_SATA_MODE = 2 : default "1022,7804" if STONEYRIDGE_SATA_MODE = 5 : : endif # STONEYRIDGE_SATA_MODE = 2 || STONEYRIDGE_SATA_MODE = 5 : : config STONEYRIDGE_LEGACY_FREE : PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/Kconfig@250 PS2, Line 250: TONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/Kconfig@257 PS2, Line 257: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/Kconfig@389 PS2, Line 389: SOC_AMD_STONEYRIDGE_FP4 || SOC_AMD_STONEYRIDGE_FT4 PICASSO, how many sockets?
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/acpi.c File src/soc/amd/picasso/acpi.c:
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/acpi.c@78 PS2, Line 78: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/acpi/globalnvs.a... File src/soc/amd/picasso/acpi/globalnvs.asl:
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/acpi/globalnvs.a... PS2, Line 20: stoneyridg picasso
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/bootblock/bootbl... File src/soc/amd/picasso/bootblock/bootblock.c:
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/bootblock/bootbl... PS2, Line 111: STONEYRIDGE_ PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/chip.h File src/soc/amd/picasso/chip.h:
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/chip.h@16 PS2, Line 16: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/chip.h@17 PS2, Line 17: STONEYRIDG PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/chip.h@30 PS2, Line 30: STONEY PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/chip.h@77 PS2, Line 77: stoneyridge picasso
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/chip.h@81 PS2, Line 81: STONEYRIDG PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/chip.c File src/soc/amd/picasso/chip.c:
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/chip.c@35 PS2, Line 35: stoneyridge picasso
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/chip.c@150 PS2, Line 150: stoneyridg picasso
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/chip.c@151 PS2, Line 151: StoneyRidge Picasso
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/acpi... File src/soc/amd/picasso/include/soc/acpi.h:
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/acpi... PS2, Line 18: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/acpi... PS2, Line 19: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/acpi... PS2, Line 23: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/acpi... PS2, Line 40: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/cpu.... File src/soc/amd/picasso/include/soc/cpu.h:
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/cpu.... PS2, Line 16: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/cpu.... PS2, Line 17: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/cpu.... PS2, Line 35: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/gpio... File src/soc/amd/picasso/include/soc/gpio.h:
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/gpio... PS2, Line 16: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/gpio... PS2, Line 17: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/gpio... PS2, Line 308: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/i2c.... File src/soc/amd/picasso/include/soc/i2c.h:
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/i2c.... PS2, Line 16: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/i2c.... PS2, Line 17: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/i2c.... PS2, Line 49: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/ioma... File src/soc/amd/picasso/include/soc/iomap.h:
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/ioma... PS2, Line 17: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/ioma... PS2, Line 18: TONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/ioma... PS2, Line 88: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/nort... File src/soc/amd/picasso/include/soc/northbridge.h:
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/nort... PS2, Line 17: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/nort... PS2, Line 18: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/nort... PS2, Line 133: STONEYRID PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/nvs.... File src/soc/amd/picasso/include/soc/nvs.h:
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/nvs.... PS2, Line 20: stoneyridge picasso
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/nvs.... PS2, Line 24: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/nvs.... PS2, Line 25: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/nvs.... PS2, Line 67: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/pci_... File src/soc/amd/picasso/include/soc/pci_devs.h:
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/pci_... PS2, Line 16: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/pci_... PS2, Line 17: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/pci_... PS2, Line 198: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/roms... File src/soc/amd/picasso/include/soc/romstage.h:
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/roms... PS2, Line 16: #ifndef __STONEYRIDGE_ROMSTAGE_H__ PICASSO all over
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/smbu... File src/soc/amd/picasso/include/soc/smbus.h:
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/smbu... PS2, Line 16: __STONEYRIDGE_SMBUS_H__ PICASSO, 3 places
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/smi.... File src/soc/amd/picasso/include/soc/smi.h:
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/smi.... PS2, Line 18: ifndef __SOUTHBRIDGE_AMD_PI_STONEYRIDGE_SMI_H__ : #define __SOUTHBRIDGE_AMD_PI_STONEYRIDGE_SMI_H__ : PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/smi.... PS2, Line 242: __SOUTHBRIDGE_AMD_PI_STONEYRIDGE_SMI_H__ PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/sout... File src/soc/amd/picasso/include/soc/southbridge.h:
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/sout... PS2, Line 17: TONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/sout... PS2, Line 18: STONEYRIDG PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/sout... PS2, Line 319: stoneyridge picasso
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/include/soc/sout... PS2, Line 415: __STONEYRIDGE_H__ PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/makefile.inc File src/soc/amd/picasso/makefile.inc:
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/makefile.inc@30 PS2, Line 30: ifeq ($(CONFIG_SOC_AMD_STONEYRIDGE_FP4)$(CONFIG_SOC_AMD_STONEYRIDGE_FT4),y) PICASSO, what socket type?
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/makefile.inc@40 PS2, Line 40: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/makefile.inc@66 PS2, Line 66: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/makefile.inc@78 PS2, Line 78: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/makefile.inc@84 PS2, Line 84: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/makefile.inc@109 PS2, Line 109: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/makefile.inc@125 PS2, Line 125: stoneyridge picasso
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/makefile.inc@126 PS2, Line 126: stoneyridge picasso
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/makefile.inc@127 PS2, Line 127: stoneyridge picasso
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/makefile.inc@138 PS2, Line 138: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/makefile.inc@218 PS2, Line 218: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/makefile.inc@219 PS2, Line 219: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/makefile.inc@238 PS2, Line 238: STONEYRIDG PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/makefile.inc@239 PS2, Line 239: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/makefile.inc@268 PS2, Line 268: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/makefile.inc@276 PS2, Line 276: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/makefile.inc@283 PS2, Line 283: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/makefile.inc@286 PS2, Line 286: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/makefile.inc@292 PS2, Line 292: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/makefile.inc@316 PS2, Line 316: endif # ($(CONFIG_SOC_AMD_STONEYRIDGE_FP4)$(CONFIG_SOC_AMD_STONEYRIDGE_FT4),y) PICASSO, what socket type?
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/northbridge.c File src/soc/amd/picasso/northbridge.c:
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/northbridge.c@28... PS2, Line 280: AGESA Will it be AGESA or FSP at the end? same in other places bellow.
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/northbridge.c@35... PS2, Line 350: family15 family17
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/northbridge.c@38... PS2, Line 388: fam1 family17
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/romstage.c File src/soc/amd/picasso/romstage.c:
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/romstage.c@110 PS2, Line 110: * TODO: This is a hack to work around current AGESA behavior. : * AGESA needs to change to reflect that coreboot owns : * the MTRRs. : * : * After setting up DRAM, AGESA also completes the configuration : * of the MTRRs, setting regions to WB. Anything written to : * memory between now and and when CAR is dismantled will be : * in cache and lost. For now, set the regions UC to ensure : * the writes get to DRAM. : */ : Really? Say it's a copy from stoney, and that needs to be reviewed in the near future, once AGESA/FSP is solved.
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/southbridge.c File src/soc/amd/picasso/southbridge.c:
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/southbridge.c@59 PS2, Line 59: STONEYRIDGE PICASSO
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/southbridge.c@60 PS2, Line 60: STONEYRIDGE same
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/southbridge.c@66 PS2, Line 66: STONEYRIDGE same
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/southbridge.c@67 PS2, Line 67: STONEYRIDG same
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/southbridge.c@73 PS2, Line 73: STONEYRIDGE same
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/southbridge.c@74 PS2, Line 74: STONEYRIDGE same
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/southbridge.c@80 PS2, Line 80: STONEYRIDGE same
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/southbridge.c@94 PS2, Line 94: STONEYRIDGE same
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/tsc_freq.c File src/soc/amd/picasso/tsc_freq.c:
https://review.coreboot.org/#/c/32407/2/src/soc/amd/picasso/tsc_freq.c@33 PS2, Line 33: amily 15h Models 70h-7F Wrong, family 17h models ?
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 2:
(108 comments) Way too many stoney or stoneyridge. I hope I found them all.
This patch is an _exact_ copy of stoneyridge, where the only difference (see the commit message) is that Makefile.inc is renamed. In this way, it's easy to review the changes that are made to it as it transforms into a working picasso. The alternative is to simply drop in a brand new directory where it's unreasonable to ask anyone to review it.
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 2:
Patch Set 2:
(108 comments) Way too many stoney or stoneyridge. I hope I found them all.
This patch is an _exact_ copy of stoneyridge, where the only difference (see the commit message) is that Makefile.inc is renamed. In this way, it's easy to review the changes that are made to it as it transforms into a working picasso. The alternative is to simply drop in a brand new directory where it's unreasonable to ask anyone to review it.
Well, at least path such as soc/amd/... has to be fixed. Also, I believe, family must be fixed.
Hello Aaron Durbin, Edward O'Callaghan, Marshall Dawson, Richard Spiegel, Paul Menzel, build bot (Jenkins), Nico Huber, Furquan Shaikh, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32407
to look at the new patch set (#3).
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
soc/amd/picasso: Create picasso as a copy of stoneyridge
So that everyone can see what's being updated from stoney, we're starting with a direct copy of the stoney directory. There are arguments both for and against doing it this way, but I believe This the most transparent way. We've moved much of the duplicated stoney code into the soc/amd/common directory and will continue that work as it becomes obvious that we have unchanged code between the SOCs.
Makefile.inc has been renamed as makefile.inc so that it won't build in jenkins until the directory is updated.
Other than that change, this is an exact copy of the stoneyridge SOC directory which will be updated in the follow-on commits in the patch train.
TEST=None BUG=b:130804851
Signed-off-by: Martin Roth martinroth@chromium.org Change-Id: I6809bd1eea304f76dd9000c079b3ed09f94dbd3b --- A src/soc/amd/picasso/BiosCallOuts.c A src/soc/amd/picasso/Kconfig A src/soc/amd/picasso/acpi.c A src/soc/amd/picasso/acpi/acpi_wake_source.asl A src/soc/amd/picasso/acpi/cpu.asl A src/soc/amd/picasso/acpi/globalnvs.asl A src/soc/amd/picasso/acpi/northbridge.asl A src/soc/amd/picasso/acpi/pci_int.asl A src/soc/amd/picasso/acpi/pcie.asl A src/soc/amd/picasso/acpi/sb_fch.asl A src/soc/amd/picasso/acpi/sb_pci0_fch.asl A src/soc/amd/picasso/acpi/sleepstates.asl A src/soc/amd/picasso/acpi/soc.asl A src/soc/amd/picasso/acpi/usb.asl A src/soc/amd/picasso/bootblock/bootblock.c A src/soc/amd/picasso/chip.c A src/soc/amd/picasso/chip.h A src/soc/amd/picasso/cpu.c A src/soc/amd/picasso/enable_usbdebug.c A src/soc/amd/picasso/finalize.c A src/soc/amd/picasso/gpio.c A src/soc/amd/picasso/i2c.c A src/soc/amd/picasso/include/soc/acpi.h A src/soc/amd/picasso/include/soc/amd_pci_int_defs.h A src/soc/amd/picasso/include/soc/cpu.h A src/soc/amd/picasso/include/soc/gpio.h A src/soc/amd/picasso/include/soc/i2c.h A src/soc/amd/picasso/include/soc/iomap.h A src/soc/amd/picasso/include/soc/northbridge.h A src/soc/amd/picasso/include/soc/nvs.h A src/soc/amd/picasso/include/soc/pci_devs.h A src/soc/amd/picasso/include/soc/romstage.h A src/soc/amd/picasso/include/soc/smbus.h A src/soc/amd/picasso/include/soc/smi.h A src/soc/amd/picasso/include/soc/southbridge.h A src/soc/amd/picasso/makefile.inc A src/soc/amd/picasso/mca.c A src/soc/amd/picasso/monotonic_timer.c A src/soc/amd/picasso/nb_util.c A src/soc/amd/picasso/northbridge.c A src/soc/amd/picasso/pmutil.c A src/soc/amd/picasso/ramtop.c A src/soc/amd/picasso/reset.c A src/soc/amd/picasso/romstage.c A src/soc/amd/picasso/sata.c A src/soc/amd/picasso/sm.c A src/soc/amd/picasso/smbus.c A src/soc/amd/picasso/smbus_spd.c A src/soc/amd/picasso/smi.c A src/soc/amd/picasso/smi_util.c A src/soc/amd/picasso/smihandler.c A src/soc/amd/picasso/southbridge.c A src/soc/amd/picasso/spi.c A src/soc/amd/picasso/tsc_freq.c A src/soc/amd/picasso/uart.c A src/soc/amd/picasso/usb.c 56 files changed, 9,198 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/32407/3
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 3:
Well, at least path such as soc/amd/... has to be fixed. Also, I believe, family must be fixed.
Yes, in the follow-on commits. I've updated the commit message to reflect this.
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 3: Code-Review+2
I see... so this is kind of "work in progress", where makefile will be renamed once conversion is completed.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 3:
Patch Set 2:
(108 comments)
Way too many stoney or stoneyridge. I hope I found them all.
So, from what I read, looks like all of these will be renamed on a follow-up?
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 3:
Patch Set 3:
Patch Set 2:
(108 comments)
Way too many stoney or stoneyridge. I hope I found them all.
So, from what I read, looks like all of these will be renamed on a follow-up?
Absolutely. Leaving them in this patch was completely intentional, and they'll be fixed up in the following patch train.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 3: Code-Review+1
Patch Set 3:
Patch Set 3:
Patch Set 2:
(108 comments)
Way too many stoney or stoneyridge. I hope I found them all.
So, from what I read, looks like all of these will be renamed on a follow-up?
Absolutely. Leaving them in this patch was completely intentional, and they'll be fixed up in the following patch train.
Ack. I would suggest tagging all the patches on the train with a common topic, so that they can be tracked more easily on Gerrit. Looking forward to having newer AMD chips in the tree :)
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 3: Code-Review+2
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 3: Code-Review+1
(14 comments)
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/Kconfig File src/soc/amd/picasso/Kconfig:
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/Kconfig@16 PS3, Line 16: STONEYRIDGE looks like it is intentional
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/chip.c File src/soc/amd/picasso/chip.c:
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/chip.c@17 PS3, Line 17: include <console/console.h> if I'm not wrong, this is not used
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/chip.c@35 PS3, Line 35: stoneyridge is it intentional?
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/chip.c@42 PS3, Line 42: stoney is it intentional?
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/enable_usbdebug.... File src/soc/amd/picasso/enable_usbdebug.c:
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/enable_usbdebug.... PS3, Line 22: <device/pci_def.h maybe not used?
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/include/soc/nort... File src/soc/amd/picasso/include/soc/northbridge.h:
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/include/soc/nort... PS3, Line 17: PI_STONEYRIDGE_NORTHBRIDGE_H ?
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/include/soc/nvs.... File src/soc/amd/picasso/include/soc/nvs.h:
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/include/soc/nvs.... PS3, Line 20: stoneyridge is it intentional?
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/include/soc/nvs.... PS3, Line 24: __SOC_STONEYRIDGE_NVS_H__ ?
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/include/soc/pci_... File src/soc/amd/picasso/include/soc/pci_devs.h:
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/include/soc/pci_... PS3, Line 16: __PI_STONEYRIDGE_PCI_DEVS_H__ : #define __PI_STONEYRIDGE_PCI_DEVS_H__ ?
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/include/soc/roms... File src/soc/amd/picasso/include/soc/romstage.h:
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/include/soc/roms... PS3, Line 16: __STONEYRIDGE_ROMSTAGE_H__ ?
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/include/soc/smbu... File src/soc/amd/picasso/include/soc/smbus.h:
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/include/soc/smbu... PS3, Line 16: _STONEYRIDGE is it intentional?
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/include/soc/smi.... File src/soc/amd/picasso/include/soc/smi.h:
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/include/soc/smi.... PS3, Line 18: SOUTHBRIDGE_AMD_PI_STONEYRIDGE ?
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/include/soc/sout... File src/soc/amd/picasso/include/soc/southbridge.h:
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/include/soc/sout... PS3, Line 17: _STONEYRIDGE ?
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/tsc_freq.c File src/soc/amd/picasso/tsc_freq.c:
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/tsc_freq.c@33 PS3, Line 33: Family 15h Models 70h-7Fh 17h ?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
Patch Set 3:
(6 comments)
THe idea is that this change is an identical copy of stoneyridge. Follow-up changes then correct everything that differs, so that one can see the difference between stoney and picasso.
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/Kconfig File src/soc/amd/picasso/Kconfig:
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/Kconfig@16 PS3, Line 16: STONEYRIDGE
looks like it is intentional
Indeed.
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/chip.c File src/soc/amd/picasso/chip.c:
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/chip.c@17 PS3, Line 17: include <console/console.h>
if I'm not wrong, this is not used
It is used. See the end of this file, which makes use of the function: void post_code(u8 value);
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/chip.c@35 PS3, Line 35: stoneyridge
is it intentional?
Yes, see recent change log messages, as well as Patchset 2.
This applies to all the comments about "stoney" being in this patch.
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/enable_usbdebug.... File src/soc/amd/picasso/enable_usbdebug.c:
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/enable_usbdebug.... PS3, Line 22: <device/pci_def.h
maybe not used?
Looks like it. Should be fixed on stoneyridge as well.
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/include/soc/nvs.... File src/soc/amd/picasso/include/soc/nvs.h:
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/include/soc/nvs.... PS3, Line 20: stoneyridge
is it intentional?
Yes.
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/tsc_freq.c File src/soc/amd/picasso/tsc_freq.c:
https://review.coreboot.org/#/c/32407/3/src/soc/amd/picasso/tsc_freq.c@33 PS3, Line 33: Family 15h Models 70h-7Fh
17h ?
The code still needs to be changed to work for picasso chips.
Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32407 )
Change subject: soc/amd/picasso: Create picasso as a copy of stoneyridge ......................................................................
soc/amd/picasso: Create picasso as a copy of stoneyridge
So that everyone can see what's being updated from stoney, we're starting with a direct copy of the stoney directory. There are arguments both for and against doing it this way, but I believe This the most transparent way. We've moved much of the duplicated stoney code into the soc/amd/common directory and will continue that work as it becomes obvious that we have unchanged code between the SOCs.
Makefile.inc has been renamed as makefile.inc so that it won't build in jenkins until the directory is updated.
Other than that change, this is an exact copy of the stoneyridge SOC directory which will be updated in the follow-on commits in the patch train.
TEST=None BUG=b:130804851
Signed-off-by: Martin Roth martinroth@chromium.org Change-Id: I6809bd1eea304f76dd9000c079b3ed09f94dbd3b Reviewed-on: https://review.coreboot.org/c/coreboot/+/32407 Reviewed-by: Richard Spiegel richard.spiegel@silverbackltd.com Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Edward O'Callaghan quasisec@chromium.org Reviewed-by: HAOUAS Elyes ehaouas@noos.fr Tested-by: build bot (Jenkins) no-reply@coreboot.org --- A src/soc/amd/picasso/BiosCallOuts.c A src/soc/amd/picasso/Kconfig A src/soc/amd/picasso/acpi.c A src/soc/amd/picasso/acpi/acpi_wake_source.asl A src/soc/amd/picasso/acpi/cpu.asl A src/soc/amd/picasso/acpi/globalnvs.asl A src/soc/amd/picasso/acpi/northbridge.asl A src/soc/amd/picasso/acpi/pci_int.asl A src/soc/amd/picasso/acpi/pcie.asl A src/soc/amd/picasso/acpi/sb_fch.asl A src/soc/amd/picasso/acpi/sb_pci0_fch.asl A src/soc/amd/picasso/acpi/sleepstates.asl A src/soc/amd/picasso/acpi/soc.asl A src/soc/amd/picasso/acpi/usb.asl A src/soc/amd/picasso/bootblock/bootblock.c A src/soc/amd/picasso/chip.c A src/soc/amd/picasso/chip.h A src/soc/amd/picasso/cpu.c A src/soc/amd/picasso/enable_usbdebug.c A src/soc/amd/picasso/finalize.c A src/soc/amd/picasso/gpio.c A src/soc/amd/picasso/i2c.c A src/soc/amd/picasso/include/soc/acpi.h A src/soc/amd/picasso/include/soc/amd_pci_int_defs.h A src/soc/amd/picasso/include/soc/cpu.h A src/soc/amd/picasso/include/soc/gpio.h A src/soc/amd/picasso/include/soc/i2c.h A src/soc/amd/picasso/include/soc/iomap.h A src/soc/amd/picasso/include/soc/northbridge.h A src/soc/amd/picasso/include/soc/nvs.h A src/soc/amd/picasso/include/soc/pci_devs.h A src/soc/amd/picasso/include/soc/romstage.h A src/soc/amd/picasso/include/soc/smbus.h A src/soc/amd/picasso/include/soc/smi.h A src/soc/amd/picasso/include/soc/southbridge.h A src/soc/amd/picasso/makefile.inc A src/soc/amd/picasso/mca.c A src/soc/amd/picasso/monotonic_timer.c A src/soc/amd/picasso/nb_util.c A src/soc/amd/picasso/northbridge.c A src/soc/amd/picasso/pmutil.c A src/soc/amd/picasso/ramtop.c A src/soc/amd/picasso/reset.c A src/soc/amd/picasso/romstage.c A src/soc/amd/picasso/sata.c A src/soc/amd/picasso/sm.c A src/soc/amd/picasso/smbus.c A src/soc/amd/picasso/smbus_spd.c A src/soc/amd/picasso/smi.c A src/soc/amd/picasso/smi_util.c A src/soc/amd/picasso/smihandler.c A src/soc/amd/picasso/southbridge.c A src/soc/amd/picasso/spi.c A src/soc/amd/picasso/tsc_freq.c A src/soc/amd/picasso/uart.c A src/soc/amd/picasso/usb.c 56 files changed, 9,198 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified HAOUAS Elyes: Looks good to me, but someone else must approve Richard Spiegel: Looks good to me, approved Angel Pons: Looks good to me, but someone else must approve Edward O'Callaghan: Looks good to me, approved
-
Angel Pons (Code Review) -
build bot (Jenkins) (Code Review)
-
Edward O'Callaghan (Code Review)
-
HAOUAS Elyes (Code Review)
-
Kyösti Mälkki (Code Review)
-
Marshall Dawson (Code Review)
-
Martin Roth (Code Review)
-
Nico Huber (Code Review)
-
Paul Menzel (Code Review)
-
Richard Spiegel (Code Review)