coreboot February 2016

coreboot@coreboot.org

31 participants
69 discussions

Re: [coreboot] RFC: coding style: "standard" defines
by Alex G. 05 Feb '16

05 Feb '16

Hi there, This seems pretty simple. On 02/04/2016 01:35 AM, Patrick Georgi via coreboot wrote: > Hi all, > > during the review of some commits that are in the process of being > upstreamed from Chrome OS, people noticed that chipset drivers like to > define their own TRUE/FALSE defines (sometimes prefixed to), and I > have seen a bunch of #define BIT{0-31} ..., too, because that seems to > be the house rules in some firmware communities. > > I think we should seek uniformity here: decide on some style, > recommend it, clean up the tree to match, and help people stay > consistent through lint tests. What I don't know however is what that > style should look like. > > So, two topics: > > 1. TRUE/FALSE > Do we want such defines? If so, TRUE/FALSE, or true/false, or > True/False, or ...? Standardize on stdbool.h, bool, true, false. It's the logical choice. > 2. BIT16 vs BIT(16) vs (1 << 16) vs 0x10000 > I don't think it makes sense to go for a single one of these (0x3ff is > certainly more readable than BIT11 | BIT10 | BIT9 | BIT8 | BIT7 | BIT8 > | BIT5 | BIT4 | BIT3 | BIT2 | BIT1 | BIT 0), but I doubt we need both > BIT16 and BIT(16). BIT16, no. Who wants 64 defines? Seriously, this sort of stuff raises blood pressure. BIT(16), sure. Actually, I used this trick a while back on the allwinner port. BIT(16) meant "just a bit", whereas (1 << 16) meant an enum (usually also paired close to 2 << 16, 3 << 16, etc). 0x10000 also fine. If it's in a list of defines, why not? #define RON 0x8000 #define JOHN 0x10000 #define MARY 0x20000 We all know Chromium guys like to copy paste code from all over the interwebs, and quite frankly, why make their lives harder with harsh guidelines? These are small inconsistencies that I don't think bother anyone too much. Alex

1 0

Re: [coreboot] RFC: coding style: "standard" defines
by Timothy Pearson 05 Feb '16

05 Feb '16

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 02/04/2016 04:22 PM, ron minnich wrote: > Nico, I agree with you completely: non-zero is true, zero false. It's C. > But I am starting now to feel like a dinosaur, b/c > "Oh, and I'm trying to hang with the cool kids and use bool :)" -- Rusty > > It makes me want to start an emo coreboot programmer feed like this > one: https://twitter.com/kylor3n?lang=en > for those of us who have been using "C as it is spoken" for more than 5 > years :-) > > Seriously, though, I agree with you but ... the world may not :-) > > ron For the record, I agree with you here. "non-zero is true, zero false". - -- Timothy Pearson Raptor Engineering +1 (415) 727-8645 (direct line) +1 (512) 690-0200 (switchboard) http://www.raptorengineeringinc.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQEcBAEBAgAGBQJWs9FWAAoJEK+E3vEXDOFbBy8H/j3UucwN4PfpNP+5RdngJ0o8 rJJvLJr6QnmR14DnnW+IIJ+4cXj+jiv+RaI+TbiC37joZXLzS5iyK8xC9rRurdrV bUTVBpMVVHNevzDF21tho5ZeH8TE75Ho1hpLqMW3XFjw4hT191K39pNCR2S++zvC B3OsF1As3hsO/Cw2QGwSXLAJBP7o6Tbth00fTYp4NFGo2VBFEBAs+1MA8artEbtv 3BiYS2VwGtfKlnQAKzve+rbc7TLWWJWYAWF1Zc7eJGXHFeDQ2gl+6m7euKsrcubd mFOOndPuRtqGi4YBVJHZRPsIqgMxqDCuebc3DkqPrVGbUQouWqwhY4PNOT13bak= =/BTX -----END PGP SIGNATURE-----

1 0

Re: [coreboot] RFC: coding style: "standard" defines
by Nico Huber 05 Feb '16

05 Feb '16

On 04.02.2016 22:25, Patrick Georgi via coreboot wrote: > 2016-02-04 22:22 GMT+01:00 Martin Roth <gaumless(a)gmail.com>: >> I don't think we need redefinitions of TRUE/FALSE > We have no canonical definitions for TRUE/FALSE right now. > Contributions that use them (for whatever reason) tend to bring local > copies, and that's what I'd like to avoid. I don't like true/false definitions neither. If we have contributions which bring them, well, we should factor it out during review. Arguments against true/false definitions? It's C! As we know, in C, everything but 0 is naturally true. While redefining true/false seems to enhance readability, it breaks with this principle. That might not be a problem if you write new code---you usually know how you want to interpret true then. However if you just read code you might find a condition like if (something == true) So, I expect that this comparison is done in terms of the type of `something` (and is only "true" if `something` evaluates to 1). Is it like that? Well, I'm pretty sure it is. But having to think about it is already too much IMO. Nico

3 2

Re: [coreboot] RFC: coding style: "standard" defines
by Nico Huber 05 Feb '16

05 Feb '16

On 04.02.2016 10:35, Patrick Georgi via coreboot wrote: > I think we should seek uniformity here: decide on some style, > recommend it, clean up the tree to match, and help people stay > consistent through lint tests. That's a good idea. > 2. BIT16 vs BIT(16) vs (1 << 16) vs 0x10000 > I don't think it makes sense to go for a single one of these (0x3ff is > certainly more readable than BIT11 | BIT10 | BIT9 | BIT8 | BIT7 | BIT8 > | BIT5 | BIT4 | BIT3 | BIT2 | BIT1 | BIT 0), but I doubt we need both > BIT16 and BIT(16). I would prefer not to have definitions for single bits. Maybe it's just the code that I've seen using this pattern, but I don't find it any way readable. If there are more than three BITS in an expression, I lose track which are set. So typography really isn't my domain, but I think the problem is the dominant BIT prefix which makes all those BITS look the same. Another argument against BIT* and BIT(x) definitions: It encourages programmers to split numbers into single bits. Something that I've seen too often: A datasheet defines some single bits and some numbers in a word. Let's say bits 30, 15, 8-5 and 1. What we get to review looks like: something |= BIT30 | BIT15 | BIT8 | BIT6 | BIT1; // Welcome to hell! Instead we could have: something |= (1 << 30) | (1 << 15) | (10 << 5) | (1 << 1); // is it really 10? (what I can't recall to have ever seen: a mix of BIT* with numbers) Nico

1 0

Re: [coreboot] RFC: coding style: "standard" defines
by ron minnich 04 Feb '16

04 Feb '16

Vendors frequently ask us to use magic numbers, and have even requested at times that we remove explanatory text. We'll never get rid of magic numbers in this world :-( ron On Thu, Feb 4, 2016 at 1:25 PM Patrick Georgi <pgeorgi(a)google.com> wrote: > 2016-02-04 22:22 GMT+01:00 Martin Roth <gaumless(a)gmail.com>: > > I don't think we need redefinitions of TRUE/FALSE > We have no canonical definitions for TRUE/FALSE right now. > Contributions that use them (for whatever reason) tend to bring local > copies, and that's what I'd like to avoid. > > > How would people feel about adding something to the coding guide to > > avoid magic numbers? > Make that a separate thread please :-) > > > Patrick > > > On Thu, Feb 4, 2016 at 6:05 AM, ron minnich <rminnich(a)gmail.com> wrote: > >> > >> > >> On Thu, Feb 4, 2016 at 2:29 AM Patrick Georgi via coreboot > >> <coreboot(a)coreboot.org> wrote: > >>> > >>> > >>> > >>> 1. TRUE/FALSE > >>> Do we want such defines? If so, TRUE/FALSE, or true/false, or > >>> True/False, or ...? > >> > >> > >> should we start using bool ...? > >>> > >>> > >>> 2. BIT16 vs BIT(16) vs (1 << 16) vs 0x10000 > >>> I don't think it makes sense to go for a single one of these (0x3ff is > >>> certainly more readable than BIT11 | BIT10 | BIT9 | BIT8 | BIT7 | BIT8 > >>> | BIT5 | BIT4 | BIT3 | BIT2 | BIT1 | BIT 0), but I doubt we need both > >>> BIT16 and BIT(16). > >>> > >> > >> BIT16 is a constant. BIT(16) is a chance for things to go badly wrong, > e.g. > >> BIT(x-y) might produce some very strange problems. I kind of prefer the > >> constant. > >> > >> ron > >> > >> -- > >> coreboot mailing list: coreboot(a)coreboot.org > >> http://www.coreboot.org/mailman/listinfo/coreboot > > > > -- > Google Germany GmbH, ABC-Str. 19, 20354 Hamburg > Registergericht und -nummer: Hamburg, HRB 86891, Sitz der Gesellschaft: > Hamburg > Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle >

1 0

Re: [coreboot] RFC: coding style: "standard" defines
by Patrick Georgi 04 Feb '16

04 Feb '16

2016-02-04 22:22 GMT+01:00 Martin Roth <gaumless(a)gmail.com>: > I don't think we need redefinitions of TRUE/FALSE We have no canonical definitions for TRUE/FALSE right now. Contributions that use them (for whatever reason) tend to bring local copies, and that's what I'd like to avoid. > How would people feel about adding something to the coding guide to > avoid magic numbers? Make that a separate thread please :-) Patrick > On Thu, Feb 4, 2016 at 6:05 AM, ron minnich <rminnich(a)gmail.com> wrote: >> >> >> On Thu, Feb 4, 2016 at 2:29 AM Patrick Georgi via coreboot >> <coreboot(a)coreboot.org> wrote: >>> >>> >>> >>> 1. TRUE/FALSE >>> Do we want such defines? If so, TRUE/FALSE, or true/false, or >>> True/False, or ...? >> >> >> should we start using bool ...? >>> >>> >>> 2. BIT16 vs BIT(16) vs (1 << 16) vs 0x10000 >>> I don't think it makes sense to go for a single one of these (0x3ff is >>> certainly more readable than BIT11 | BIT10 | BIT9 | BIT8 | BIT7 | BIT8 >>> | BIT5 | BIT4 | BIT3 | BIT2 | BIT1 | BIT 0), but I doubt we need both >>> BIT16 and BIT(16). >>> >> >> BIT16 is a constant. BIT(16) is a chance for things to go badly wrong, e.g. >> BIT(x-y) might produce some very strange problems. I kind of prefer the >> constant. >> >> ron >> >> -- >> coreboot mailing list: coreboot(a)coreboot.org >> http://www.coreboot.org/mailman/listinfo/coreboot -- Google Germany GmbH, ABC-Str. 19, 20354 Hamburg Registergericht und -nummer: Hamburg, HRB 86891, Sitz der Gesellschaft: Hamburg Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle

1 0

Re: [coreboot] RFC: coding style: "standard" defines
by Martin Roth 04 Feb '16

04 Feb '16

I don't think we need redefinitions of TRUE/FALSE Ron's argument against using BIT(16) makes sense to me. I don't have a problem with BIT16, 0x10000, or (1 << 16) but #define VENDOR_BIT_16 0x10000 seems like overkill. I'd really generally prefer some good definitions though instead of write(BIT16) I'd really like to see: #define ENABLE_FEATURE BIT16 write(ENABLE_FEATURE) I know that at times we don't have good documentation about what the values actually do, but we've got a lot of magic numbers throughout our code. How would people feel about adding something to the coding guide to avoid magic numbers? Martin On Thu, Feb 4, 2016 at 6:05 AM, ron minnich <rminnich(a)gmail.com> wrote: > > > On Thu, Feb 4, 2016 at 2:29 AM Patrick Georgi via coreboot > <coreboot(a)coreboot.org> wrote: >> >> >> >> 1. TRUE/FALSE >> Do we want such defines? If so, TRUE/FALSE, or true/false, or >> True/False, or ...? > > > should we start using bool ...? >> >> >> 2. BIT16 vs BIT(16) vs (1 << 16) vs 0x10000 >> I don't think it makes sense to go for a single one of these (0x3ff is >> certainly more readable than BIT11 | BIT10 | BIT9 | BIT8 | BIT7 | BIT8 >> | BIT5 | BIT4 | BIT3 | BIT2 | BIT1 | BIT 0), but I doubt we need both >> BIT16 and BIT(16). >> > > BIT16 is a constant. BIT(16) is a chance for things to go badly wrong, e.g. > BIT(x-y) might produce some very strange problems. I kind of prefer the > constant. > > ron > > -- > coreboot mailing list: coreboot(a)coreboot.org > http://www.coreboot.org/mailman/listinfo/coreboot

1 0

Re: [coreboot] ASUS KGPE-D16 Automated Test Failure
by Timothy Pearson 04 Feb '16

04 Feb '16

On 02/04/2016 11:38 AM, Raptor Engineering Automated Coreboot Test Stand wrote: > The ASUS KGPE-D16 fails verification as of commit 4af905ac95685500e71fb32cf5cec430d1a75447 > > The following tests failed: > BOOT_FAILURE Taking the KGPE-D16 offline until we can get this sorted; too many failures in too short of a timespan. Sorry for the noise! -- Timothy Pearson Raptor Engineering +1 (415) 727-8645 (direct line) +1 (512) 690-0200 (switchboard) http://www.raptorengineeringinc.com

1 0

Re: [coreboot] bootfail on my Mohon Peak CRB.
by WANG FEI 04 Feb '16

04 Feb '16

If your platform still does not boot up, I would suggest you check the SoC revision on your platform with the microcode revision of your coreboot, these two revisions should be matched, if not, system will halt in a every early stage. Another possibility you should consider is that if the UART configuration of your board, if it's wrong, you definitely will not have any output on serial console. On Thu, Feb 4, 2016 at 2:02 PM, WANG FEI <wangfei.jimei(a)gmail.com> wrote: > *RANGELEY_POSTGOLD4_FSP_004_20150924.fd is the FSP binary, you can rename > it to FvFsp.bin and placed it to the path defined in coreboot, ie, * > `../intel/fsp/rangeley/Fv*, to generate coreboot image.* > > On Thu, Feb 4, 2016 at 5:19 AM, 김유석 <poplinux0(a)gmail.com> wrote: > >> Dear Martin. >> >> Thank's your advise. >> >> I'm use the serial consol port. but can't see any message. >> >> Thank you. >> >> 2016-02-02 오후 9:18에 Martin Roth 이(가) 쓴 글: >> >> You might try a different video card. There's an issue in the video bios >> of the aspeed card that came with the mohon peak. >> >> martin >> On Tue, Feb 2, 2016 at 00:41 김유석 < <poplinux0(a)gmail.com> >> poplinux0(a)gmail.com> wrote: >> >>> Dear sir. >>> >>> My ENV is see below. >>> >>> *EVB : Intel rangeley Mohon Peak CRB* >>> >>> >>> This time, I was download the coreboot from git. >>> >>> poplinux@raw work $ > git clone >>> <http://review.coreboot.org/coreboot.git> >>> http://review.coreboot.org/coreboot.git ./ >>> poplinux@raw work $ > cd coreboot >>> poplinux@raw coreboot $ > git submodule update --init --checkout >>> >>> Next, *run make menuconfig* and set-up to mohon peak CRB and save & exit >>> >>> * Mainboa**rd* >>> Mainboard vendor (*Intel*) ---> >>> Mainboard model (*Mohon Peak CRB*) ---> >>> [ ] Configure defaults for the Intel FSP package >>> ROM chip size (2048 KB (2 MB)) ---> >>> (0x00200000) Size of CBFS filesystem in ROM >>> () fmap description file in fmd format >>> >>> Next, I'm try to build core boot. >>> >>> poplinux@raw coreboot $ > make >>> GEN generated/bootblock.ld >>> CP bootblock/arch/x86/bootblock.ld >>> LINK cbfs/fallback/bootblock.debug >>> OBJCOPY cbfs/fallback/bootblock.elf >>> OBJCOPY bootblock.raw.bin >>> Checking out SeaBIOS revision >>> 01a84bea2d28a19d2405c1ecac4bdef17683cc0c >>> Switched to branch 'master' >>> >>> Performing operation on 'COREBOOT' region... >>> Name Offset Type Size >>> cbfs master header 0x0 cbfs header 32 >>> fallback/romstage 0x80 stage 22684 >>> cpu_microcode_blob.bin 0x5980 microcode 0 >>> config 0x5a00 raw 127 >>> revision 0x5ac0 raw 570 >>> cmos_layout.bin 0x5d40 cmos_layout 1316 >>> fallback/dsdt.aml 0x62c0 raw 7952 >>> payload_config 0x8240 raw 1574 >>> payload_revision 0x88c0 raw 237 >>> (empty) 0x8a00 null 29848 >>> mrc.cache 0xfec0 mrc_cache 65536 >>> fallback/ramstage 0x1ff00 stage 46922 >>> fallback/payload 0x2b6c0 payload 61122 >>> (empty) 0x3a5c0 null 1856216 >>> bootblock 0x1ff8c0 bootblock 1528 >>> >>> Finally, I'm got a coreboot image. >>> >>> >>> poplinux@raw build $ > ls build/coreboot.rom >>> build/coreboot.rom >>> poplinux@raw build $ > ./build/cbfstool build/coreboot.rom print >>> Performing operation on 'COREBOOT' region... >>> Name Offset Type Size >>> cbfs master header 0x0 cbfs header 32 >>> fallback/romstage 0x80 stage 22684 >>> cpu_microcode_blob.bin 0x5980 microcode 0 >>> config 0x5a00 raw 127 >>> revision 0x5ac0 raw 570 >>> cmos_layout.bin 0x5d40 cmos_layout 1316 >>> fallback/dsdt.aml 0x62c0 raw 7952 >>> payload_config 0x8240 raw 1574 >>> payload_revision 0x88c0 raw 237 >>> (empty) 0x8a00 null 29848 >>> mrc.cache 0xfec0 mrc_cache 65536 >>> fallback/ramstage 0x1ff00 stage 46922 >>> fallback/payload 0x2b6c0 payload 61122 >>> (empty) 0x3a5c0 null 1856216 >>> bootblock 0x1ff8c0 bootblock 1528 >>> >>> >>> And I'm write image to my EVB using *ALL-100 Gang-writ**er*. >>> spi flash's write *start address is set 0x00000000*. write it success. >>> >>> And I'm attach the flash memory to my EVB. >>> >>> And power-up the my EVB. But can't see any message on my monitor and >>> serial port both. >>> >>> >>> *Why did not display any message? * >>> *A**nd could you support correct configuration file for my EVB?* >>> >>> Thank you. >>> >>> >>> >>> >>> >>> >>> -- >>> coreboot mailing list: coreboot(a)coreboot.org >>> http://www.coreboot.org/mailman/listinfo/coreboot >> >> >> >> -- >> coreboot mailing list: coreboot(a)coreboot.org >> http://www.coreboot.org/mailman/listinfo/coreboot >> > >

1 0

Re: [coreboot] bootfail on my Mohon Peak CRB.
by 김유석 04 Feb '16

04 Feb '16

Dear Martin. Thank's your advise. I'm use the serial consol port. but can't see any message. Thank you. 2016-02-02 오후 9:18에 Martin Roth 이(가) 쓴 글: > You might try a different video card. There's an issue in the video > bios of the aspeed card that came with the mohon peak. > > martin > On Tue, Feb 2, 2016 at 00:41 김유석 <poplinux0(a)gmail.com > <mailto:poplinux0@gmail.com>> wrote: > > Dear sir. > > My ENV is see below. > > *EVB : Intel rangeley Mohon Peak CRB* > > > This time, I was download the coreboot from git. > > poplinux@raw work $ > git clone > http://review.coreboot.org/coreboot.git ./ > poplinux@raw work $ > cd coreboot > poplinux@raw coreboot $ > git submodule update --init --checkout > > Next, *run make menuconfig* and set-up to mohon peak CRB and save > & exit > > *Mainboa**rd* > Mainboard vendor (*Intel*) ---> > Mainboard model (*Mohon Peak CRB*) ---> > [ ] Configure defaults for the Intel FSP package > ROM chip size (2048 KB (2 MB)) ---> > (0x00200000) Size of CBFS filesystem in ROM > () fmap description file in fmd format > > Next, I'm try to build core boot. > > poplinux@raw coreboot $ > make > GEN generated/bootblock.ld > CP bootblock/arch/x86/bootblock.ld > LINK cbfs/fallback/bootblock.debug > OBJCOPY cbfs/fallback/bootblock.elf > OBJCOPY bootblock.raw.bin > Checking out SeaBIOS revision > 01a84bea2d28a19d2405c1ecac4bdef17683cc0c > Switched to branch 'master' > > Performing operation on 'COREBOOT' region... > Name Offset Type Size > cbfs master header 0x0 cbfs header 32 > fallback/romstage 0x80 stage 22684 > cpu_microcode_blob.bin 0x5980 microcode 0 > config 0x5a00 raw 127 > revision 0x5ac0 raw 570 > cmos_layout.bin 0x5d40 cmos_layout 1316 > fallback/dsdt.aml 0x62c0 raw 7952 > payload_config 0x8240 raw 1574 > payload_revision 0x88c0 raw 237 > (empty) 0x8a00 null 29848 > mrc.cache 0xfec0 mrc_cache 65536 > fallback/ramstage 0x1ff00 stage 46922 > fallback/payload 0x2b6c0 payload 61122 > (empty) 0x3a5c0 null 1856216 > bootblock 0x1ff8c0 bootblock 1528 > > Finally, I'm got a coreboot image. > > > poplinux@raw build $ > ls build/coreboot.rom > build/coreboot.rom > poplinux@raw build $ > ./build/cbfstool build/coreboot.rom print > Performing operation on 'COREBOOT' region... > Name Offset Type Size > cbfs master header 0x0 cbfs header 32 > fallback/romstage 0x80 stage 22684 > cpu_microcode_blob.bin 0x5980 microcode 0 > config 0x5a00 raw 127 > revision 0x5ac0 raw 570 > cmos_layout.bin 0x5d40 cmos_layout 1316 > fallback/dsdt.aml 0x62c0 raw 7952 > payload_config 0x8240 raw 1574 > payload_revision 0x88c0 raw 237 > (empty) 0x8a00 null 29848 > mrc.cache 0xfec0 mrc_cache 65536 > fallback/ramstage 0x1ff00 stage 46922 > fallback/payload 0x2b6c0 payload 61122 > (empty) 0x3a5c0 null 1856216 > bootblock 0x1ff8c0 bootblock 1528 > > > And I'm write image to my EVB using *ALL-100 Gang-writ**er*. > spi flash's write *start address is set 0x00000000*. write it success. > > And I'm attach the flash memory to my EVB. > > And power-up the my EVB. But can't see any message on my monitor > and serial port both. > > > *Why did not display any message? ** > **A**nd could you support correct configuration file for my EVB?* > > Thank you. > > > > > > > -- > coreboot mailing list: coreboot(a)coreboot.org > <mailto:coreboot@coreboot.org> > http://www.coreboot.org/mailman/listinfo/coreboot >

2 1

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

2003

2002

coreboot February 2016