Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Rizwan Qureshi, Subrata Banik, Patrick Rudolph.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55965 )
Change subject: arch/x86: Helper functions to get deterministic cache parameters
......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS4:
> @Angel, i hope you meant only 2 functions as
>
> 1. fill_cpu_cache_info
> 2. get_cache_size
>
> and discard all other functions ?
I meant to keep the current functions, and add another that returns a struct as Tim suggested.
--
To view, visit https://review.coreboot.org/c/coreboot/+/55965
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0dd701fb47460092448b64c7fa2162f762bf3095
Gerrit-Change-Number: 55965
Gerrit-PatchSet: 4
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Wed, 07 Jul 2021 21:24:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Maulik V Vaghela, John Zhao.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56024 )
Change subject: drivers/usb/acpi: Create function to get PLD information
......................................................................
Patch Set 4: Code-Review+1
(1 comment)
File src/drivers/usb/acpi/usb_acpi.c:
https://review.coreboot.org/c/coreboot/+/56024/comment/1873b51d_f6397b3f
PS4, Line 135: if (config->use_custom_pld) {
: /* Use board defined PLD */
: memcpy(&pld, &config->custom_pld, sizeof(pld));
: } else {
: /* Fill PLD strucutre based on port type */
: acpi_pld_fill_usb(&pld, config->type, &config->group);
: }
nit: I don't think the braces or comments are necessary
--
To view, visit https://review.coreboot.org/c/coreboot/+/56024
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iaaf140ce1965dce3a812aa2701ce0e29b34ab3e7
Gerrit-Change-Number: 56024
Gerrit-PatchSet: 4
Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: John Zhao <john.zhao(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Abhijeet Rao <abhijeet.rao(a)intel.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Attention: John Zhao <john.zhao(a)intel.com>
Gerrit-Comment-Date: Wed, 07 Jul 2021 21:19:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Rizwan Qureshi, Angel Pons, Patrick Rudolph.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55965 )
Change subject: arch/x86: Helper functions to get deterministic cache parameters
......................................................................
Patch Set 4:
(1 comment)
File src/arch/x86/include/arch/cpu.h:
https://review.coreboot.org/c/coreboot/+/55965/comment/27272722_c9d9a929
PS4, Line 325: CACHE_L0
> What's a "L0" cache?
Its not L0 cache, rather its like ECX=0, i don;t find any nicer way to convey that, ideally we can send ECX=0 to n unless you read EAX.Cache Level = 0 as return. Also at the same time, want to make sure that the caller can pass the argument say's what is 3 while calling cpuid_ext(4, 4)
--
To view, visit https://review.coreboot.org/c/coreboot/+/55965
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0dd701fb47460092448b64c7fa2162f762bf3095
Gerrit-Change-Number: 55965
Gerrit-PatchSet: 4
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Wed, 07 Jul 2021 21:16:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Rizwan Qureshi, Subrata Banik, Patrick Rudolph.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55965 )
Change subject: arch/x86: Helper functions to get deterministic cache parameters
......................................................................
Patch Set 4:
(2 comments)
Patchset:
PS4:
> This is really bice suggestion, i was thinking to create smaller sets of function where things are i […]
I'd suggest having both the functions and the struct
File src/arch/x86/include/arch/cpu.h:
https://review.coreboot.org/c/coreboot/+/55965/comment/60cbd31f_35817893
PS4, Line 325: CACHE_L0
What's a "L0" cache?
--
To view, visit https://review.coreboot.org/c/coreboot/+/55965
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0dd701fb47460092448b64c7fa2162f762bf3095
Gerrit-Change-Number: 55965
Gerrit-PatchSet: 4
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Wed, 07 Jul 2021 21:10:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Rizwan Qureshi, Angel Pons, Patrick Rudolph.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55965 )
Change subject: arch/x86: Helper functions to get deterministic cache parameters
......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS4:
> I wonder if having a struct that encompasses this information would lead to a little less code, e,g. […]
This is really bice suggestion, i was thinking to create smaller sets of function where things are independent enough and caller can call those routines based on need. But your point is also valid, that, there may be less chance that some one would like to know what is number of cache sets or its being fully associative or not, so its better to fill all those CPUID EAX=leaf, ECX={0/1/2/3..} output into a single structure as pass to the caller to use it wisely ?
--
To view, visit https://review.coreboot.org/c/coreboot/+/55965
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0dd701fb47460092448b64c7fa2162f762bf3095
Gerrit-Change-Number: 55965
Gerrit-PatchSet: 4
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Wed, 07 Jul 2021 21:03:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Rizwan Qureshi, Subrata Banik, Angel Pons, Patrick Rudolph.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55965 )
Change subject: arch/x86: Helper functions to get deterministic cache parameters
......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS4:
I wonder if having a struct that encompasses this information would lead to a little less code, e,g.:
```
struct cpu_cache_info {
unsigned int type;
unsigned int level;
unsigned int num_ways;
unsigned int num_sets;
unsigned int line_size;
unsigned int size;
unsigned int physical_partitions;
unsigned int num_cores_shared;
bool fully_associative;
};
```
or something similar? Then there can just be a function that maybe accepts a pointer to that type and all of the helpers and caching goes away, e.g.:
```
void fill_cpu_cache_info(struct cpu_cache_info *info)
{
struct cpuid_result result;
if (!info)
return;
get_leaf();
info->type = ...
etc;
}
```
WDYT?
--
To view, visit https://review.coreboot.org/c/coreboot/+/55965
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0dd701fb47460092448b64c7fa2162f762bf3095
Gerrit-Change-Number: 55965
Gerrit-PatchSet: 4
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Wed, 07 Jul 2021 20:57:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Marshall Dawson, Paul Menzel, Felix Held.
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55857 )
Change subject: soc/amd/common/block/lpc/spi_dma: Implement readat_async
......................................................................
Patch Set 8:
(1 comment)
File src/soc/amd/common/block/lpc/spi_dma.c:
https://review.coreboot.org/c/coreboot/+/55857/comment/6b556f5d_96e60043
PS5, Line 34: * can always move the number to a Kconfig if necessary. */
> Please use one of the recommended multi-line comment styles [1]. […]
lol, oops/
--
To view, visit https://review.coreboot.org/c/coreboot/+/55857
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0be555956581fd82bbe1482d8afa8828c61aaa00
Gerrit-Change-Number: 55857
Gerrit-PatchSet: 8
Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Wed, 07 Jul 2021 20:24:42 +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: Jason Glenesk, Marshall Dawson, Paul Menzel, Felix Held.
Hello build bot (Jenkins), Jason Glenesk, Marshall Dawson, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/55857
to look at the new patch set (#8).
Change subject: soc/amd/common/block/lpc/spi_dma: Implement readat_async
......................................................................
soc/amd/common/block/lpc/spi_dma: Implement readat_async
This change implements the region_device async readat API. The
implementation is a simple state machine that iterates over a queue
starting new transactions whenever the previous one has completed.
There is a magic bit that needs to be set for the SPI DMA controller to
function correctly. This is only available in RN/CZN+.
BUG=b:179699789
TEST=Boot guybrush to OS
Signed-off-by: Raul E Rangel <rrangel(a)chromium.org>
Change-Id: I0be555956581fd82bbe1482d8afa8828c61aaa00
---
M src/soc/amd/common/block/include/amdblocks/lpc.h
M src/soc/amd/common/block/lpc/spi_dma.c
2 files changed, 287 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/55857/8
--
To view, visit https://review.coreboot.org/c/coreboot/+/55857
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0be555956581fd82bbe1482d8afa8828c61aaa00
Gerrit-Change-Number: 55857
Gerrit-PatchSet: 8
Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newpatchset