Attention is currently required from: Felix Held, Fred Reitberger, Martin Roth, Nikolai Vyssotski, Paul Menzel.
Hello Felix Held, Fred Reitberger, Martin Roth, Nikolai Vyssotski, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/81887?usp=email
to look at the new patch set (#4).
Change subject: soc/amd/glinda: Add support for A0 and B0 steppings
......................................................................
soc/amd/glinda: Add support for A0 and B0 steppings
Update the A0 and B0 stepping IDs in CPU table per
the PPR document 57254 Rev 1.56 and 1.69
Change-Id: I0072f25f981ac7d5df2522594c8788bfabcbf24c
Signed-off-by: Anand Vaikar <a.vaikar2021(a)gmail.com>
---
M src/soc/amd/glinda/cpu.c
M src/soc/amd/glinda/include/soc/cpu.h
2 files changed, 4 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/81887/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/81887?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I0072f25f981ac7d5df2522594c8788bfabcbf24c
Gerrit-Change-Number: 81887
Gerrit-PatchSet: 4
Gerrit-Owner: Anand Vaikar <a.vaikar2021(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: ritul guru <ritul.bits(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com>
Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Held, Fred Reitberger, Martin Roth, Nikolai Vyssotski, Paul Menzel.
Anand Vaikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81887?usp=email )
Change subject: soc/amd/glinda: Add support for A0 and B0 steppings
......................................................................
Patch Set 3:
(4 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/81887/comment/94c575ab_bfbabe6d :
PS2, Line 7: soc/amd/glinda :
> Please remove the space before the colon.
Done
https://review.coreboot.org/c/coreboot/+/81887/comment/241435df_622fcd24 :
PS2, Line 9: Update the A0 and B0 stepping IDs in CPU table
> Please mention the datasheet name and revision.
Done
File src/soc/amd/glinda/include/soc/cpu.h:
https://review.coreboot.org/c/coreboot/+/81887/comment/4b96c6ca_bf769f4d :
PS2, Line 6: 0x1A
> Lowercase?
Done
https://review.coreboot.org/c/coreboot/+/81887/comment/e6c7eace_3eab5a69 :
PS2, Line 6:
> Use tabs?
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/81887?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I0072f25f981ac7d5df2522594c8788bfabcbf24c
Gerrit-Change-Number: 81887
Gerrit-PatchSet: 3
Gerrit-Owner: Anand Vaikar <a.vaikar2021(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: ritul guru <ritul.bits(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com>
Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Thu, 18 Apr 2024 08:53:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: comment
Attention is currently required from: Anand Vaikar, Felix Held, Fred Reitberger, Martin Roth, Nikolai Vyssotski.
Hello Felix Held, Fred Reitberger, Martin Roth, Nikolai Vyssotski, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/81887?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: soc/amd/glinda: Add support for A0 and B0 steppings
......................................................................
soc/amd/glinda: Add support for A0 and B0 steppings
Update the A0 and B0 stepping IDs in CPU table per
the PPR document 57254 Rev 1.56 and 1.59
Change-Id: I0072f25f981ac7d5df2522594c8788bfabcbf24c
Signed-off-by: Anand Vaikar <a.vaikar2021(a)gmail.com>
---
M src/soc/amd/glinda/cpu.c
M src/soc/amd/glinda/include/soc/cpu.h
2 files changed, 4 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/81887/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/81887?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I0072f25f981ac7d5df2522594c8788bfabcbf24c
Gerrit-Change-Number: 81887
Gerrit-PatchSet: 3
Gerrit-Owner: Anand Vaikar <a.vaikar2021(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: ritul guru <ritul.bits(a)gmail.com>
Gerrit-Attention: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com>
Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Anand Vaikar <a.vaikar2021(a)gmail.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: Jian Tong, Paul Menzel, Shelley Chen.
Eric Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81945?usp=email )
Change subject: mb/google/brox: Move hda verb to variant dir
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS5:
> cim_verb_datas and pc_beep_verbs is used directly by azalia_device. […]
Acknowledged
--
To view, visit https://review.coreboot.org/c/coreboot/+/81945?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Id987c248c37dc8bdc63be7a2513fa8997b5ddc33
Gerrit-Change-Number: 81945
Gerrit-PatchSet: 5
Gerrit-Owner: Jian Tong <tongjian(a)huaqin.corp-partner.google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Poornima Tom <poornima.tom(a)intel.com>
Gerrit-Reviewer: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Shelley Chen <shchen(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jian Tong <tongjian(a)huaqin.corp-partner.google.com>
Gerrit-Comment-Date: Thu, 18 Apr 2024 08:05:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Eric Lai <ericllai(a)google.com>
Comment-In-Reply-To: Jian Tong <tongjian(a)huaqin.corp-partner.google.com>
Gerrit-MessageType: comment
Attention is currently required from: Jian Tong, Paul Menzel, Shelley Chen.
Eric Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81945?usp=email )
Change subject: mb/google/brox: Move hda verb to variant dir
......................................................................
Patch Set 5: Code-Review+2
(1 comment)
Patchset:
PS5:
I see, it's also const...
--
To view, visit https://review.coreboot.org/c/coreboot/+/81945?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Id987c248c37dc8bdc63be7a2513fa8997b5ddc33
Gerrit-Change-Number: 81945
Gerrit-PatchSet: 5
Gerrit-Owner: Jian Tong <tongjian(a)huaqin.corp-partner.google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Poornima Tom <poornima.tom(a)intel.com>
Gerrit-Reviewer: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Shelley Chen <shchen(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jian Tong <tongjian(a)huaqin.corp-partner.google.com>
Gerrit-Comment-Date: Thu, 18 Apr 2024 08:05:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Eric Lai, Paul Menzel, Shelley Chen.
Jian Tong has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81945?usp=email )
Change subject: mb/google/brox: Move hda verb to variant dir
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS5:
> No, like what we did for gpio table override.
cim_verb_datas and pc_beep_verbs is used directly by azalia_device.c->void azalia_codecs_init(u8 *base, u16 codec_mask).
I don't thinks its a good idea to create a new API.
--
To view, visit https://review.coreboot.org/c/coreboot/+/81945?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Id987c248c37dc8bdc63be7a2513fa8997b5ddc33
Gerrit-Change-Number: 81945
Gerrit-PatchSet: 5
Gerrit-Owner: Jian Tong <tongjian(a)huaqin.corp-partner.google.com>
Gerrit-Reviewer: Poornima Tom <poornima.tom(a)intel.com>
Gerrit-Reviewer: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Eric Lai <ericllai(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Shelley Chen <shchen(a)google.com>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Thu, 18 Apr 2024 07:56:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Eric Lai <ericllai(a)google.com>
Comment-In-Reply-To: Jian Tong <tongjian(a)huaqin.corp-partner.google.com>
Gerrit-MessageType: comment
Hello Zheng Bao, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/75682?usp=email
to look at the new patch set (#2).
Change subject: amdfwtool: Add PSP backup directory for A/B recovery
......................................................................
amdfwtool: Add PSP backup directory for A/B recovery
Change-Id: Ia5a745c86595554c83cd13a35c312c17987b716b
Signed-off-by: Zheng Bao <fishbaozi(a)gmail.com>
---
M util/amdfwtool/amdfwtool.c
M util/amdfwtool/amdfwtool.h
2 files changed, 16 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/75682/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/75682?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ia5a745c86595554c83cd13a35c312c17987b716b
Gerrit-Change-Number: 75682
Gerrit-PatchSet: 2
Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-Reviewer: Zheng Bao
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Elyes Haouas, Jianeng Ceng, Paul Menzel, Subrata Banik.
Dolan Liu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81773?usp=email )
Change subject: drivers/i2c/rt5645: Add RT5645 amp driver
......................................................................
Patch Set 23:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/81773/comment/147e519e_1d550de4 :
PS9, Line 9: Add RT5645 AMP support.
> The reason is the newly added pin not support in generic I2C driver. […]
Pual,
yep, it's smiliar to rt5663, but the rt5663 is very old and it was used the hard code like `RT53` or `10EC5663`, there are so much old device used it and no longer updates in recent years, we don't wanna change it and make something others trouble for that.
on the others side, rt5663 and RT5645/5650 are the different series, it may caused some ambiguity. And if we change that to the common driver, the name should like `RT56XX`, it may cause some confusion on DT too.
because I2C generic driver dose not support dsd gpio setting, we declared the new rt5645 series driver for expansion.
--
To view, visit https://review.coreboot.org/c/coreboot/+/81773?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I602fcc4dd8576043943f6e20884edc4703350320
Gerrit-Change-Number: 81773
Gerrit-PatchSet: 23
Gerrit-Owner: Jianeng Ceng <cengjianeng(a)huaqin.corp-partner.google.com>
Gerrit-Reviewer: Dolan Liu <liuyong5(a)huaqin.corp-partner.google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jianeng Ceng <cengjianeng(a)huaqin.corp-partner.google.com>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Comment-Date: Thu, 18 Apr 2024 07:21:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Eric Lai <ericllai(a)google.com>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Comment-In-Reply-To: Jianeng Ceng <cengjianeng(a)huaqin.corp-partner.google.com>
Gerrit-MessageType: comment
Attention is currently required from: Julius Werner, Jérémy Compostella, Kapil Porwal, Nico Huber, Subrata Banik.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81960?usp=email )
Change subject: arch/x86: Enable long mode entry into payload for x86_64 support
......................................................................
Patch Set 1:
(1 comment)
File src/arch/x86/boot.c:
https://review.coreboot.org/c/coreboot/+/81960/comment/96c7b3d9_d3f3a525 :
PS1, Line 25: if (CONFIG(PAYLOAD_X86_64_SUPPORT)) {
> > > > > AIUI, the payload handover and the coreboot tables are the most important
> > > > > ABI pieces of coreboot. Making this a compile-time option would mean that
> > > > > the resulting coreboot is suddenly incompatible to all prior (x86) payload
> > > > > builds. So, shouldn't this be decided at runtime, maybe based on information
> > > > > from CBFS?
> > > >
> > > > Is it not a fact that using a 64-bit toolchain to compile the libpayload is also a static piece of information? If so, then why are we not allowing a configuration to also enter LB in the desired 64-bit mode instead of limiting it to 32-bit?
> > > >
> > > > Currently, the default/non-configurable decision about LB entry is always in protected mode.
> > > >
> > > > This CL allows for one more option where 64-bit direct entry is also possible, hence all of the following options are now valid:
> > > >
> > > > 1. 64-bit CB / 32-bit LB (using protected mode)
> > > > 2. 64-bit CB / 64-bit LB (using long mode)
> > > > 3. 32-bit CB / 32-bit LB
> > > >
> > > > If we are relying on the runtime information about coreboot operational mode and using this information to decide whether to jump into LB, then wouldn't that limit #1 (where a protected mode entry into the libpayload is not possible while using 64-bit coreboot)? Perhaps not everyone wishes to use a 64-bit payload, so it would be better to keep those selections static based on the selection of payload mode.
> > >
> > > If you are thinking that coreboot in 64-bit mode would break the compatibility with other 32-bit mode payload if we are selecting this Kconfig explicitly then we can choose two below means
> > >
> > > 1. choose this Kconfig from mainboard rather than SOC directly
> > > 2. allow this kconfig only select for chromeos booting (hence, only applies to depthcharge payload)
> >
> > I agree with Nico on this. You don't want a buildtime configurable ABI. In the past you could take any coreboot image and slap any payload on there. Also the ability to chainload payloads is an important feature. Now sometimes things break a little and a 10y old payload might not work with a modern coreboot or vise versa. I think a lot of us just build coreboot and payload together and never look back. However I still think there is value in that model, so lets not throw that away. Also the whole universal payload stuff is exactly about defining an ABI + payload info standard :-).
> >
> > It's ok to break the ABI in the sense that there is a new way to jump to a 64bit payload directly for 64bit coreboot, but that should be decided at runtime: i.e. the payload should have a new cbfs attribute to inform coreboot or the chainloading payload about the ABI. coreboot then needs multiple codepaths to deal with the different options and that's fine IMO. That way also chainloading 32bit payloads or 32bit coreboot could in principle load a 64bit payload.
>
> noted! sounds like a mixmatch requirement for post processing the binary for in-field device which may/may not be the motivation for device manufacture to support. I value your opinion here.
>
> Can you please let me know what is the way to add new cbfs attribute into the payload/libpayload here to tell that coreboot should hand-off to the libpayload in 64-bit mode rather trunking?
So there would be 2 ways: Add the information in a new 'cbfs_payload_segment_type'. cbfstool would parse the elf and add that information in there. A different way would be in cbfs_file_attr_tag. To remain backwards compatible the lack of the cbfs attribute would mean the payload is x86 32bit.
--
To view, visit https://review.coreboot.org/c/coreboot/+/81960?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ic5e6f0af11c05e8b075b8c20880c012747a1df9b
Gerrit-Change-Number: 81960
Gerrit-PatchSet: 1
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Comment-Date: Thu, 18 Apr 2024 07:09:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment