coreboot-gerrit February 2020

coreboot-gerrit@coreboot.org

1 participants
1796 discussions

Change in coreboot[master]: soc/intel/apollolake: Display apollolake platform information
by Paul Menzel (Code Review) 17 Feb '20

17 Feb '20

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38824 ) Change subject: soc/intel/apollolake: Display apollolake platform information ...................................................................... Patch Set 4: (7 comments) For the future the name mapping should be done differently. Some text file, or use of some database, so it does not need to be in code, and even duplicated. https://review.coreboot.org/c/coreboot/+/38824/4//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/38824/4//COMMIT_MSG@7 PS4, Line 7: apollolake Not needed, as it’s in the prefix. https://review.coreboot.org/c/coreboot/+/38824/4//COMMIT_MSG@9 PS4, Line 9: apollolake Apollo Lake https://review.coreboot.org/c/coreboot/+/38824/4//COMMIT_MSG@19 PS4, Line 19: Geminilake It’s the successor of Apollo Lake. https://review.coreboot.org/c/coreboot/+/38824/4/src/soc/intel/apollolake/r… File src/soc/intel/apollolake/report_platform.c: https://review.coreboot.org/c/coreboot/+/38824/4/src/soc/intel/apollolake/r… PS4, Line 79: u32 i The counting variable should be just `unsigned int i`. https://review.coreboot.org/c/coreboot/+/38824/4/src/soc/intel/apollolake/r… PS4, Line 81: int vt, txt, aes; Why not boolean type? Also, rename to `vt_supported`, and so on. https://review.coreboot.org/c/coreboot/+/38824/4/src/soc/intel/apollolake/r… PS4, Line 102: BIOS_DEBUG I’d make that BIOS_INFO. `src/commonlib/include/commonlib/loglevel.h` has definitions, but it does not really apply to informational messages. https://review.coreboot.org/c/coreboot/+/38824/4/src/soc/intel/apollolake/r… PS4, Line 117: int unsigned int -- To view, visit https://review.coreboot.org/c/coreboot/+/38824 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id4edfeae7faee9f5f80698cf34b31fdcb066a813 Gerrit-Change-Number: 38824 Gerrit-PatchSet: 4 Gerrit-Owner: Usha P <usha.p(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Usha P <usha.p(a)intel.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Mon, 17 Feb 2020 11:59:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/apollolake: Display apollolake platform information
by Usha P (Code Review) 17 Feb '20

17 Feb '20

Hello Werner Zeh, Patrick Rudolph, Subrata Banik, Aamir Bohra, Rizwan Qureshi, build bot (Jenkins), Patrick Georgi, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/38824 to look at the new patch set (#4). Change subject: soc/intel/apollolake: Display apollolake platform information ...................................................................... soc/intel/apollolake: Display apollolake platform information This patch includes the change required to display apollolake platform information which reports CPU, MCH, PCH and IGD information in romstage. BUG=None TEST= 1. Boot to OS on Bobba board. 2. Verified below info from CPU Console log in romstage CPU: Intel(R) Celeron(R) N4000 CPU @ 1.10GHz CPU: ID 706a1, Geminilake B0, ucode: 00000031 CPU: AES supported, TXT NOT supported, VT supported MCH: device id 31f0 (rev 03) is Geminilake PCH: device id 3197 (rev 03) is Geminilake IGD: device id 3185 (rev 03) is Geminilake EU12 Change-Id: Id4edfeae7faee9f5f80698cf34b31fdcb066a813 Signed-off-by: Usha P <usha.p(a)intel.com> --- M src/soc/intel/apollolake/Makefile.inc M src/soc/intel/apollolake/include/soc/romstage.h A src/soc/intel/apollolake/report_platform.c M src/soc/intel/apollolake/romstage.c 4 files changed, 177 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/38824/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/38824 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id4edfeae7faee9f5f80698cf34b31fdcb066a813 Gerrit-Change-Number: 38824 Gerrit-PatchSet: 4 Gerrit-Owner: Usha P <usha.p(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Usha P <usha.p(a)intel.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/common/block/lpc: create LPC_GET_DEV macro
by Subrata Banik (Code Review) 17 Feb '20

17 Feb '20

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30209 ) Change subject: soc/intel/common/block/lpc: create LPC_GET_DEV macro ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/c/coreboot/+/30209/2/src/soc/intel/common/block… File src/soc/intel/common/block/lpc/lpc_lib.c: https://review.coreboot.org/c/coreboot/+/30209/2/src/soc/intel/common/block… PS2, Line 29: #if !defined(__SIMPLE_DEVICE__) : #include <device/device.h> : #define LPC_GET_DEV pcidev_on_root(PCH_DEV_SLOT_LPC, 0x0) : #else : #define LPC_GET_DEV PCH_DEV_LPC : #endif > > > Compiles fine as by Aamir's suggestion, btw. > > > > yes compilation will work as long as #define __SIMPLE_DEVICE__ is there. > > No, I meant this patch but with PCH_DEV_LPC from soc/pci_devs.h instead > of your new macro works fine. Do you mean PCH_DEV_LPC from soc/pci_devs.h will work without having #define __SIMPLE_DEVICE__ at line 18? > > > > > But i don't know why we have this guard at first place and want to compile this entire file as part of simple device. is that saving anything ? general query. > > It provides a stable API even if the file is compiled for ramstage and > pre-ram stages. Intention is not to break the stability here for sure, just wondering if you have real reason why #define __SIMPLE_DEVICE__ been added at first place ? > Hence, avoids the need for any #if in platform code. FYI, this is soc code not the platform code. hatch, poppy are the platform code in my understanding. this is soc library. i was adhering old review comments from Arthur who has suggested to have a macro rather function names. > because you can't use them anymore to get a `struct device *`. That's > why there are some explicit calls to pcidev_on_root() below. yes, that's the point. this could have been avoided but it has some other assumptions :) about "struct device *" -- To view, visit https://review.coreboot.org/c/coreboot/+/30209 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I84a6102bf3849e9d4fe28e4c6a11bc7badcf5114 Gerrit-Change-Number: 30209 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 17 Feb 2020 09:52:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Comment-In-Reply-To: Aamir Bohra <aamir.bohra(a)intel.com> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Test Only
by Alan Lee (Code Review) 17 Feb '20

17 Feb '20

Alan Lee has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38938 ) Change subject: Test Only ...................................................................... Test Only Signed-off-by: Alan Lee <alan_lee(a)compal.corp-partner.google.com> Change-Id: Id49c8cd84697412da3426cba2f4d95939718aff9 Signed-off-by: Alan Lee <alan_lee(a)compal.corp-partner.google.com> --- M src/mainboard/google/drallion/variants/drallion/Makefile.inc 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/38938/1 diff --git a/src/mainboard/google/drallion/variants/drallion/Makefile.inc b/src/mainboard/google/drallion/variants/drallion/Makefile.inc index b584a91..033d0fd 100644 --- a/src/mainboard/google/drallion/variants/drallion/Makefile.inc +++ b/src/mainboard/google/drallion/variants/drallion/Makefile.inc @@ -25,7 +25,7 @@ SPD_SOURCES += micron_dimm_MT40A1G16KD-062EE # 0b11010 SPD_SOURCES += hynix_dimm_H5AN8G6NDJR-XNC # 0b01100 SPD_SOURCES += samsung_dimm_K4AAG165WA-BCWE # 0b00000 -SPD_SOURCES += samsung_dimm_K4A8G165WC-BCWE # 0b00100 +SPD_SOURCES += samsung_dimm_K4A8G165WC-BCWE # 0b00101 bootblock-y += gpio.c ramstage-y += gpio.c -- To view, visit https://review.coreboot.org/c/coreboot/+/38938 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id49c8cd84697412da3426cba2f4d95939718aff9 Gerrit-Change-Number: 38938 Gerrit-PatchSet: 1 Gerrit-Owner: Alan Lee <alan_lee(a)compal.corp-partner.google.com> Gerrit-MessageType: newchange

1 1

Change in coreboot[master]: soc/intel/common/block/lpc: create LPC_GET_DEV macro
by Nico Huber (Code Review) 17 Feb '20

17 Feb '20

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30209 ) Change subject: soc/intel/common/block/lpc: create LPC_GET_DEV macro ...................................................................... Patch Set 2: Code-Review-1 (1 comment) https://review.coreboot.org/c/coreboot/+/30209/2/src/soc/intel/common/block… File src/soc/intel/common/block/lpc/lpc_lib.c: https://review.coreboot.org/c/coreboot/+/30209/2/src/soc/intel/common/block… PS2, Line 29: #if !defined(__SIMPLE_DEVICE__) : #include <device/device.h> : #define LPC_GET_DEV pcidev_on_root(PCH_DEV_SLOT_LPC, 0x0) : #else : #define LPC_GET_DEV PCH_DEV_LPC : #endif > > Compiles fine as by Aamir's suggestion, btw. > > yes compilation will work as long as #define __SIMPLE_DEVICE__ is there. No, I meant this patch but with PCH_DEV_LPC from soc/pci_devs.h instead of your new macro works fine. > > But i don't know why we have this guard at first place and want to compile this entire file as part of simple device. is that saving anything ? general query. It provides a stable API even if the file is compiled for ramstage and pre-ram stages. Hence, avoids the need for any #if in platform code. It makes it harder, though, to have common definitions like PCH_DEV_LPC, because you can't use them anymore to get a `struct device *`. That's why there are some explicit calls to pcidev_on_root() below. -- To view, visit https://review.coreboot.org/c/coreboot/+/30209 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I84a6102bf3849e9d4fe28e4c6a11bc7badcf5114 Gerrit-Change-Number: 30209 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 17 Feb 2020 09:42:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Comment-In-Reply-To: Aamir Bohra <aamir.bohra(a)intel.com> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common/block/lpc: create LPC_GET_DEV macro
by Subrata Banik (Code Review) 17 Feb '20

17 Feb '20

1 0

Change in coreboot[master]: soc/intel/common/block/lpc: create LPC_GET_DEV macro
by Subrata Banik (Code Review) 17 Feb '20

17 Feb '20

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30209 ) Change subject: soc/intel/common/block/lpc: create LPC_GET_DEV macro ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/c/coreboot/+/30209/2/src/soc/intel/common/block… File src/soc/intel/common/block/lpc/lpc_lib.c: https://review.coreboot.org/c/coreboot/+/30209/2/src/soc/intel/common/block… PS2, Line 29: #if !defined(__SIMPLE_DEVICE__) : #include <device/device.h> : #define LPC_GET_DEV pcidev_on_root(PCH_DEV_SLOT_LPC, 0x0) : #else : #define LPC_GET_DEV PCH_DEV_LPC : #endif > Compiles fine as by Aamir's suggestion, btw. yes compilation will work as long as #define __SIMPLE_DEVICE__ is there. But i don't know why we have this guard at first place and want to compile this entire file as part of simple device. is that saving anything ? general query. > > That you had to get rid of the declaration in line 179, doesn't > mean we need yet another macro. I believe this new macro help it to compile as per need, i mean till romstage as simple device else regularly. unlike entire file is compile as simple device (again i don't know the goodness or bad about this) also another reason to pick this is p2sb.c is also has same problem where we have redundant function name which can be avoided. infact looking at old southbridge code that arthur has written has same implementation. Also in some AMD latest common code. -- To view, visit https://review.coreboot.org/c/coreboot/+/30209 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I84a6102bf3849e9d4fe28e4c6a11bc7badcf5114 Gerrit-Change-Number: 30209 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 17 Feb 2020 09:27:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Comment-In-Reply-To: Aamir Bohra <aamir.bohra(a)intel.com> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common/block/lpc: create LPC_GET_DEV macro
by Nico Huber (Code Review) 17 Feb '20

17 Feb '20

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30209 ) Change subject: soc/intel/common/block/lpc: create LPC_GET_DEV macro ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/c/coreboot/+/30209/2/src/soc/intel/common/block… File src/soc/intel/common/block/lpc/lpc_lib.c: https://review.coreboot.org/c/coreboot/+/30209/2/src/soc/intel/common/block… PS2, Line 29: #if !defined(__SIMPLE_DEVICE__) : #include <device/device.h> : #define LPC_GET_DEV pcidev_on_root(PCH_DEV_SLOT_LPC, 0x0) : #else : #define LPC_GET_DEV PCH_DEV_LPC : #endif > Why do you want to remove the `#define __SIMPLE_DEVICE__`? It's been […] Compiles fine as by Aamir's suggestion, btw. That you had to get rid of the declaration in line 179, doesn't mean we need yet another macro. -- To view, visit https://review.coreboot.org/c/coreboot/+/30209 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I84a6102bf3849e9d4fe28e4c6a11bc7badcf5114 Gerrit-Change-Number: 30209 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 17 Feb 2020 09:14:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Comment-In-Reply-To: Aamir Bohra <aamir.bohra(a)intel.com> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common/block/lpc: create LPC_GET_DEV macro
by Nico Huber (Code Review) 17 Feb '20

17 Feb '20

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30209 ) Change subject: soc/intel/common/block/lpc: create LPC_GET_DEV macro ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/c/coreboot/+/30209/2/src/soc/intel/common/block… File src/soc/intel/common/block/lpc/lpc_lib.c: https://review.coreboot.org/c/coreboot/+/30209/2/src/soc/intel/common/block… PS2, Line 29: #if !defined(__SIMPLE_DEVICE__) : #include <device/device.h> : #define LPC_GET_DEV pcidev_on_root(PCH_DEV_SLOT_LPC, 0x0) : #else : #define LPC_GET_DEV PCH_DEV_LPC : #endif > right now we have a […] Why do you want to remove the `#define __SIMPLE_DEVICE__`? It's been used here exactly the way it was intended to be used. `#if defined(__SIMPLE_DEVICE__)` is discouraged, OTOH. -- To view, visit https://review.coreboot.org/c/coreboot/+/30209 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I84a6102bf3849e9d4fe28e4c6a11bc7badcf5114 Gerrit-Change-Number: 30209 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 17 Feb 2020 09:07:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Comment-In-Reply-To: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common/block/lpc: create LPC_GET_DEV macro
by Subrata Banik (Code Review) 17 Feb '20

17 Feb '20

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30209 ) Change subject: soc/intel/common/block/lpc: create LPC_GET_DEV macro ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/c/coreboot/+/30209/2/src/soc/intel/common/block… File src/soc/intel/common/block/lpc/lpc_lib.c: https://review.coreboot.org/c/coreboot/+/30209/2/src/soc/intel/common/block… PS2, Line 29: #if !defined(__SIMPLE_DEVICE__) : #include <device/device.h> : #define LPC_GET_DEV pcidev_on_root(PCH_DEV_SLOT_LPC, 0x0) : #else : #define LPC_GET_DEV PCH_DEV_LPC : #endif > would this be inherently taken care by: […] right now we have a #define __SIMPLE_DEVICE__ guard at line 18 https://review.coreboot.org/c/coreboot/+/30209/2/src/soc/intel/common/block… this tells all PCH_DEV_LPC reference to go to simple device check in entire file. problem is this file only complies to simple_device as we have guard above. after deleting tha guard, ramstage has started giving error as below (because of fetching code from !__SIMPLE_DEVICE__ in pch_devs.h) CC ramstage/soc/intel/common/block/lpc/lpc_lib.o In file included from src/soc/intel/common/block/lpc/lpc_lib.c:27: src/soc/intel/common/block/lpc/lpc_lib.c: In function 'lpc_set_bios_control_reg': src/soc/intel/cannonlake/include/soc/pci_devs.h:26:30: error: initialization of 'pci_devfn_t' {aka 'unsigned in '} from 'struct device *' makes integer from pointer without a cast [-Werror=int-conversion] #define _PCH_DEV(slot, func) pcidev_path_on_root_debug(_PCH_DEVFN(slot, func), __func__) ^~~~~~~~~~~~~~~~~~~~~~~~~ src/soc/intel/cannonlake/include/soc/pci_devs.h:187:23: note: in expansion of macro '_PCH_DEV' #define PCH_DEV_LPC _PCH_DEV(LPC, 0) ^~~~~~~~ src/soc/intel/common/block/lpc/lpc_lib.c:179:20: note: in expansion of macro 'PCH_DEV_LPC' pci_devfn_t dev = PCH_DEV_LPC; ^~~~~~~~~~~ src/soc/intel/common/block/lpc/lpc_lib.c:183:29: error: passing argument 1 of 'pci_read_config8' makes pointer from integer without a cast [-Werror=int-conversion] bc_cntl = pci_read_config8(dev, LPC_BIOS_CNTL); ^~~ -- To view, visit https://review.coreboot.org/c/coreboot/+/30209 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I84a6102bf3849e9d4fe28e4c6a11bc7badcf5114 Gerrit-Change-Number: 30209 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 17 Feb 2020 08:32:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2020