Attention is currently required from: Jérémy Compostella, Martin L Roth.
Pratikkumar V Prajapati has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76890?usp=email )
Change subject: Makefile.inc: Fix typo in FILANAME in comment
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/76890?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I96388245df406e6b4cb1cd3418f6a32d5b23499f
Gerrit-Change-Number: 76890
Gerrit-PatchSet: 2
Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com>
Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Comment-Date: Thu, 03 Aug 2023 17:19:48 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Frans Hendriks, Jeremy Soller, Michael Niewöhner, Michał Żygowski, Nico Huber, Piotr Król, Sean Rhodes, Tim Crawford.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75888?usp=email )
Change subject: mb/{skl,kbl}: Use true/false keywords for non-array dt options
......................................................................
Patch Set 5:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/75888/comment/53a16a4d_0c0ccc2e :
PS5, Line 13: While on it, remove the quotes from the option name and from the value.
> The quotes are actually optional. They are only needed if the values contains spaces.
Ah, interesting. The inconsistency is rather annoying, and it's likely that people will keep using quotes.
As for using true/false instead of 0/1, +2
--
To view, visit https://review.coreboot.org/c/coreboot/+/75888?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iff063c37a093e597c6b73a583903ce5e4f698856
Gerrit-Change-Number: 75888
Gerrit-PatchSet: 5
Gerrit-Owner: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Erik van den Bogaert <ebogaert(a)eltan.com>
Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-Comment-Date: Thu, 03 Aug 2023 17:17:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Elyes Haouas, Martin L Roth.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/72510?usp=email )
Change subject: coreboot-jenkins-node/Dockerfile: Upgrade lua5 from lua5.3 to lua5.4
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/72510/comment/36f199e9_86a6e2cd :
PS1, Line 7: coreboot-jenkins-node/Dockerfile: Upgrade lua5 from lua5.3 to lua5.4
> Did you test if this works? The Docker containers are not built by Jenkins.
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/72510?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic1450f0fa8eb69273aa907dea2eba8f7e7131ef1
Gerrit-Change-Number: 72510
Gerrit-PatchSet: 2
Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Comment-Date: Thu, 03 Aug 2023 17:14:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-MessageType: comment
Attention is currently required from: Martin L Roth, Tom Hiller.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/67160?usp=email )
Change subject: util/docker/coreboot-sdk: add mrc extraction packages
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/67160/comment/6d9bd940_99dcded3 :
PS2, Line 10: immage
typo nit: `image`
--
To view, visit https://review.coreboot.org/c/coreboot/+/67160?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I81ed4ef55f0ba745a8a0a0cc85c2b00360f59297
Gerrit-Change-Number: 67160
Gerrit-PatchSet: 2
Gerrit-Owner: Tom Hiller <thrilleratplay(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Tom Hiller <thrilleratplay(a)gmail.com>
Gerrit-Comment-Date: Thu, 03 Aug 2023 17:13:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Martin L Roth.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76707?usp=email )
Change subject: util/docker: refactor out and fix docker cache dir test
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/76707?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic5e1d28110097eb502959e81bafe77faa0fc7fae
Gerrit-Change-Number: 76707
Gerrit-PatchSet: 1
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Comment-Date: Thu, 03 Aug 2023 17:11:34 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Christian Walter, Jonathan Zhang, Krystian Hebel, Martin L Roth, Martin Roth, Michał Żygowski.
Sergii Dmytruk has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/67057?usp=email )
Change subject: drivers/ipmi: add BT interface
......................................................................
Patch Set 10:
(7 comments)
File src/drivers/ipmi/Kconfig:
https://review.coreboot.org/c/coreboot/+/67057/comment/f729c03c_244258a9 :
PS10, Line 68:
: config BMC_BT_BASE
> Would this be better as a register in devicetree? Since this already needs a devicetree entry, that […]
As far as I remember this option wasn't present in the first version, but there was some issue with getting this information out of devtree in ramstage (worked, but needed some workaround) and it might have not worked in romstage at all, which resulted in adding an analogue of `BMC_KCS_BASE`, which I initially thought wasn't necessary.
https://review.coreboot.org/c/coreboot/+/67057/comment/f844e10a_c5ec14ca :
PS10, Line 77: IPMI_BT_TIMEOUT_MS
> Can this be combined with the IPMI_KCS_TIMEOUT_MS value? Rename it to IPMI_TIMEOUT_MS or something?
It can because implementations are mutually exclusive. However, this would break uses of `IPMI_KCS_TIMEOUT_MS` in existing configs. If that's OK, I'll merge the options.
File src/drivers/ipmi/ipmi_bt.c:
https://review.coreboot.org/c/coreboot/+/67057/comment/541f65b2_9489de3e :
PS10, Line 58: if ((inb(port + BT_CTRL_INC) & H_BUSY) == 0)
> Do you actually need to check first? It looks like either way you go to the next line and wait for B […]
I agree, the check seems unnecessary.
https://review.coreboot.org/c/coreboot/+/67057/comment/187894b6_eb8bf857 :
PS10, Line 134: i < len
> Add a check to make sure the response isn't longer than MAX_SIZE? Since it's coming from a periphera […]
It can't be longer, length is a single byte, which is why `MAX_SIZE` is 255.
File src/drivers/ipmi/ipmi_bt_ops.c:
https://review.coreboot.org/c/coreboot/+/67057/comment/0cc6d167_41d107a4 :
PS10, Line 66: bmc_revision_minor
> the minor revision isn't allowed to be zero?
It is, probably a mistake while editing a conditional. Thanks.
File src/drivers/ipmi/ipmi_ops_premem.c:
https://review.coreboot.org/c/coreboot/+/67057/comment/740d9fc0_17aa9d01 :
PS10, Line 12: #if CONFIG(IPMI_BT)
> We generally shouldn't have to ifdef out included header files. […]
Done
https://review.coreboot.org/c/coreboot/+/67057/comment/d7d9d93e_feefe09a :
PS10, Line 57: #if CONFIG(IPMI_BT)
: if (ipmi_bt_clear(dev->path.pnp.port))
> With the header above always included, you should be able to turn this into a regular if instead of […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/67057?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idb67972d1c38bbae04c7b4de3405350c229a05b9
Gerrit-Change-Number: 67057
Gerrit-PatchSet: 10
Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Comment-Date: Thu, 03 Aug 2023 17:09:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Martin L Roth <gaumless(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Martin L Roth.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76706?usp=email )
Change subject: util/docker: Update coreboot-sdk-test to coreboot-jenkins-test
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/76706?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0e6282bbb163064f177c8e68e7180ba2bdc101f1
Gerrit-Change-Number: 76706
Gerrit-PatchSet: 1
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Comment-Date: Thu, 03 Aug 2023 17:06:55 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Christian Walter, Jonathan Zhang, Krystian Hebel, Martin Roth, Michał Żygowski, Sergii Dmytruk.
Hello Christian Walter, Jonathan Zhang, Krystian Hebel, Martin Roth, Michał Żygowski, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/67057?usp=email
to look at the new patch set (#11).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: drivers/ipmi: add BT interface
......................................................................
drivers/ipmi: add BT interface
Unlike already implemented Keyboard Controller Style (KCS) interface
Block Transfer interface is not byte-oriented and implies that device is
capable of buffering command before processing it. Another difference
is that polling can be replaced with interrupts, though not used by this
implementation.
Change-Id: Idb67972d1c38bbae04c7b4de3405350c229a05b9
Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
---
A Documentation/drivers/ipmi_bt.md
M src/drivers/ipmi/Kconfig
M src/drivers/ipmi/Makefile.inc
A src/drivers/ipmi/ipmi_bt.c
A src/drivers/ipmi/ipmi_bt.h
A src/drivers/ipmi/ipmi_bt_ops.c
M src/drivers/ipmi/ipmi_ops_premem.c
7 files changed, 403 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/67057/11
--
To view, visit https://review.coreboot.org/c/coreboot/+/67057?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idb67972d1c38bbae04c7b4de3405350c229a05b9
Gerrit-Change-Number: 67057
Gerrit-PatchSet: 11
Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Angel Pons, Frans Hendriks, Jeremy Soller, Michael Niewöhner, Michał Żygowski, Nico Huber, Piotr Król, Sean Rhodes, Tim Crawford.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75888?usp=email )
Change subject: mb/{skl,kbl}: Use true/false keywords for non-array dt options
......................................................................
Patch Set 5:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/75888/comment/7d2a6be0_95d16f1d :
PS5, Line 13: While on it, remove the quotes from the option name and from the value.
> Huh, how does this work? Is there special support in SCONFIG? Currently, it looks rather odd because […]
The quotes are actually optional. They are only needed if the values contains spaces.
--
To view, visit https://review.coreboot.org/c/coreboot/+/75888?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iff063c37a093e597c6b73a583903ce5e4f698856
Gerrit-Change-Number: 75888
Gerrit-PatchSet: 5
Gerrit-Owner: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Erik van den Bogaert <ebogaert(a)eltan.com>
Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-Comment-Date: Thu, 03 Aug 2023 17:02:12 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment