80/100/132 column line width policy
by Carl-Daniel Hailfinger
Hi,
we're hitting the 80 column limit in our code in ways which actually
reduce readability for the code. Examples are various multiline messages
and complicated nested code where refactoring to a separate function
doesn't make sense.
Keeping the old 80 column limit is not really an option anymore.
Standard terminal sizes have one of 80, 100 or 132 columns.
Given the monitor resolutions many people have nowadays, I think it is
safe to say that you can fit two xterms with 100 columns horizonally
next to each other. 100 columns should also be sufficient for a msg_p*
of roughly 80 columns of text.
132 columns provide more leeway, but IMHO that would be too wide for
good readability (and my screen can't fit two xterms side-by-side anymore).
Of course some files have sections where any column limit is not
acceptable (board lists etc.), but the column limit violations should be
limited to the affected file sections, not whole files.
Comments?
I'd like to get this decided today or tomorrow so we know where we need
line breaks in Stefan Tauner's new struct flashchip patch.
Regards,
Carl-Daniel
--
http://www.hailfinger.org/
6 months, 1 week
[PATCH] Fix undefined behavior in OS defines
by Stefan Tauner
Found by clang (warning introduced in r258128), reported by Idwer.
Signed-off-by: Stefan Tauner <stefan.tauner(a)alumni.tuwien.ac.at>
---
platform.h | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/platform.h b/platform.h
index c5a52ef..b58c303 100644
--- a/platform.h
+++ b/platform.h
@@ -24,10 +24,24 @@
#ifndef __PLATFORM_H__
#define __PLATFORM_H__ 1
-// Helper defines for operating systems
-#define IS_LINUX (defined(__gnu_linux__) || defined(__linux__))
-#define IS_MACOSX (defined(__APPLE__) && defined(__MACH__)) /* yes, both. */
-#define IS_WINDOWS (defined(_WIN32) || defined(_WIN64) || defined(__WIN32__) || defined(__WINDOWS__))
+/* Helper defines for operating systems.
+ * This may seem complicated but the following would be undefined behavior:
+ * #define IS_LINUX (defined(__gnu_linux__) || defined(__linux__)) */
+#if defined(__gnu_linux__) || defined(__linux__)
+ #define IS_LINUX (1)
+#else
+ #define IS_LINUX (0)
+#endif
+#if defined(__APPLE__) && defined(__MACH__) /* yes, both. */
+ #define IS_MACOSX (1)
+#else
+ #define IS_MACOSX (0)
+#endif
+#if defined(_WIN32) || defined(_WIN64) || defined(__WIN32__) || defined(__WINDOWS__)
+ #define IS_WINDOWS (1)
+#else
+ #define IS_WINDOWS (0)
+#endif
// Likewise for target architectures
#if defined (__i386__) || defined (__x86_64__) || defined(__amd64__)
--
Kind regards, Stefan Tauner
1 year, 8 months
flashrom chip read error
by Florin
Hello,
I got this error when trying to read an MX 25 chip using flashrom:
[florin@stick flashrom]$ sudo ./flashrom -p ch341a_spi -r ~/Desktop/bios.bin
flashrom v0.9.9-r1954 on Linux 4.10.11-1-ARCH (x86_64)
flashrom is free software, get the source code at https://flashrom.org
Calibrating delay loop... OK.
^[[AFound Generic flash chip "unknown SPI chip (RDID)" (0 kB, SPI) on
ch341a_spi.
===
This flash part has status NOT WORKING for operations: PROBE READ ERASE
WRITE
The test status of this chip may have been updated in the latest development
version of flashrom. If you are running the latest development version,
please email a report to flashrom(a)flashrom.org if any of the above
operations
work correctly for you with this flash chip. Please include the flashrom log
file for all operations you tested (see the man page for details), and
mention
which mainboard or programmer you tested in the subject line.
Thanks for your help!
Read is not working on this chip. Aborting.
[florin@stick flashrom]$ sudo ./flashrom -p ch341a_spi -r ~/Desktop/bios.bin
flashrom v0.9.9-r1954 on Linux 4.10.11-1-ARCH (x86_64)
flashrom is free software, get the source code at https://flashrom.org
Calibrating delay loop... ^[[AOK.
Found Generic flash chip "unknown SPI chip (RDID)" (0 kB, SPI) on
ch341a_spi.
===
This flash part has status NOT WORKING for operations: PROBE READ ERASE
WRITE
The test status of this chip may have been updated in the latest development
version of flashrom. If you are running the latest development version,
please email a report to flashrom(a)flashrom.org if any of the above
operations
work correctly for you with this flash chip. Please include the flashrom log
file for all operations you tested (see the man page for details), and
mention
which mainboard or programmer you tested in the subject line.
Thanks for your help!
Read is not working on this chip. Aborting.
[florin@stick flashrom]$ sudo ./flashrom -p ch341a_spi -r ~/Desktop/bios.bin
flashrom v0.9.9-r1954 on Linux 4.10.11-1-ARCH (x86_64)
flashrom is free software, get the source code at https://flashrom.org
Calibrating delay loop... OK.
Found Generic flash chip "unknown SPI chip (RDID)" (0 kB, SPI) on
ch341a_spi.
===
This flash part has status NOT WORKING for operations: PROBE READ ERASE
WRITE
The test status of this chip may have been updated in the latest development
version of flashrom. If you are running the latest development version,
please email a report to flashrom(a)flashrom.org if any of the above
operations
work correctly for you with this flash chip. Please include the flashrom log
file for all operations you tested (see the man page for details), and
mention
which mainboard or programmer you tested in the subject line.
Thanks for your help!
Read is not working on this chip. Aborting.
Thank you very much!
2 years, 1 month
Spansion S25FL128P......X
by The Raven
I have a spansion S25FL128P......X chip and can do some tests.
The "problem" is that i don't know if its an 0 or an 1.
On the chip i see only "FL128PIF" and one line lower i see "00299012 C".
Probing works (id1 0x01, id2 0x2018):
Calibrating delay loop... OK.
serprog: Programmer name is "serprog-duino"
Found Spansion flash chip "S25FL128P......0" (16384 kB, SPI) on serprog.
Found Spansion flash chip "S25FL128P......1" (16384 kB, SPI) on serprog.
Found Spansion flash chip "S25FL128S......0" (16384 kB, SPI) on serprog.
Found Spansion flash chip "S25FL128S......1" (16384 kB, SPI) on serprog.
Found Spansion flash chip "S25FL129P......0" (16384 kB, SPI) on serprog.
Found Spansion flash chip "S25FL129P......1" (16384 kB, SPI) on serprog.
Multiple flash chip definitions match the detected chip(s):
"S25FL128P......0", "S25FL128P......1", "S25FL128S......0",
"S25FL128S......1", "S25FL129P......0", "S25FL129P......1"
Please specify which chip definition to use with the -c <chipname> option.
BTW: Chip was fund on a Dell-Systemboard.
2 years, 1 month
Re: [flashrom] Namespace prefix for libflashrom
by Antonio Ospite
On Sun, 23 Apr 2017 15:54:50 -0700
David Hendricks <david.hendricks(a)gmail.com> wrote:
> On Sun, Apr 23, 2017 at 7:07 AM, Stefan Tauner <stefan.tauner(a)gmx.at> wrote:
>
> > On Sat, 22 Apr 2017 11:16:18 -0700
> > David Hendricks <david.hendricks(a)gmail.com> wrote:
[...]
> > I'd love to hear some arguments about your preferences.
>
>
> I'd be fine with libflashrom_. Since it seems there is no definitive
> favorite short prefix I think we should just forget about those and use a
> longer one. And in the grand scheme of things neither libflashrom_ nor
> flashrom_ are very long.
>
After hearing the arguments on the list I too formed an opinion about
the matter, my preference list is:
- flashrom_ (the lib prefix is redundant IMHO, lot of libraries omit
it in the API, e.g.: libglib, libgtk);
- libflashrom_ (I would tolerate it, it's still better than cryptic
prefixes);
and maybe one I didn't see:
- libfr_ (flash-rom is two words, so "fr" as abbreviation makes sense,
but there's really no need to be so concise).
Ciao ciao,
Antonio
--
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
2 years, 1 month
Re: [flashrom] Namespace prefix for libflashrom
by Stefan Tauner
On Sat, 22 Apr 2017 11:16:18 -0700
David Hendricks <david.hendricks(a)gmail.com> wrote:
> Thanks for getting this discussion going on the list, Nico.
>
> For reference, folks can view the proposed libflashrom.h at
> https://review.coreboot.org/#/c/17946 to get a better idea for how these
> prefixes will look in libflashrom functions and data structures.
>
> Also, let's add "fi_" for flashrom interface to the list of proposed
> prefixes.
>
> My preferences (in order):
> 1. lf_
> 2. flashrom_
> 3. fi_
> 4. fl_
> 5. flash_
> 6. fr_
>
> IMO not only is "lf_" most intuitive, but the way the keys are spaced apart
> comfortably on qwerty, dvorak, and colemak layouts and each character
> (including the underscore) use a different hand to type. Same could be said
> about fl_ with regards to keyboard layout. fr_ is awkward (keys vertically
> adjacent) on qwerty and colemak, and fi_ is vertically adjacent on dvorak.
> flash_ and flashrom_ are not bad but are obviously many more keystrokes.
I value giving thoughts to details, but in this case I reside more with
torvalds: "if your typing speed is the main issue when you're coding,
you're doing something seriously wrong."
In my opinion the prefix has to satisfy a number of requirements:
- no ISO C(99/11) or POSIX violations
- no easy collisions with other libraries or user functions
- easily recognizable prefix for humans associated with the concept of
libflashrom (i.e. some kind of abstraction to interact with NOR
flash chips via different programmers)
- reasonable short length to avoid overly long function names etc.
based on the prefix
> flash_ is pretty good - For the most part it flows well with functions such
> as flash_image_read() and flash_image_write(), but is awkward with some
> other stuff like "flash_set_log_callback()". If we're already typing >2
> letters I think we ought to just use flashrom_ as the prefix to be
> complete, avoid awkward contexts, and avoid namespace conflicts (users
> might want to use flash_ in their code).
flash_ seems to me a bit too concrete... it gets (too) easily associated
with the actual memory device and not so much to libflashrom IMHO.
flashrom_ is quite long and would be associated with flashrom (the
utility) and would make it harder to distinguishing the library from
the program.
libflashrom_ is probably too long but would at least transport the
meaning best in my opinion.
So... shorter but conveying the same meaning:
- lflashrom, lfrom: meh
- lflashr: would require a new awful website with flash chips dancing
for no good reason ;)
- lf: was already mentioned, I deem it a bit too short and ambiguous
- fl: too easily misinterpreted as a flashrom function prefix IMHO -
the aspect of being a library is important to me.
- libf: would be in the tradition of short-named unix libraries (and
seems to be unused actually as a library name), but does not really
improve on lf and would make it look way too important than it is :)
- lfr: was proposed in the wiki, possibly by myself - can't remember.
This might be the optimal compromise: intuitive if you heard it
once, probably quite unique and very short.
- lfl: might also work if neither lfr nor lf are acceptable (but why
would lfl then be :)
Thus my favorites are basically
- lfr_
- libflashrom_
- lf_
I'd love to hear some arguments about your preferences.
Side note because I don't have the time yet to do proper reviews but I
have briefly looked at nico's libflashrom.h:
> struct flashprog;
> typedef struct flashprog fl_flashprog_t;
> struct flashctx;
> typedef struct flashctx fl_flashctx_t;
Please avoid type abstraction with no good reason (i.e. if there are
no alternative implementations that need to be abstracted). Also, please
avoid the _t suffix, which is reserved by POSIX (the broadness and
widespread infringement of this reservation are debatable but is nuked
anyway if there is not typedef to begin with ;).
There is nothing wrong with struct fl_programmer and struct fl_context
or struct fl_ctx... or whatever the prefix is. Actually, I deem the
former superior to "flashprog" which could mean a number of things e.g.
in
int fl_flash_probe(fl_flashctx_t **, const fl_flashprog_t *, const char *chip_name);
I can come up with at least these meanings: program, programmer, progress.
Another side note:
> Moving forward is currently more important than finding the perfect name.
With that attitude you (plural) are destined to create just another
fork(-ish branch). From what I gathered up so far from the gerrit
activity this is possible happening already anyway and it might
actually be successful (although unintended nand/nor ill-intended and I
don't blame you). I will try to give some more feedback soon.
--
Kind regards/Mit freundlichen Grüßen, Stefan Tauner
2 years, 1 month
Re: [flashrom] Namespace prefix for libflashrom
by Nico Huber
On 22.04.2017 20:16, David Hendricks wrote:
> Thanks for getting this discussion going on the list, Nico.
>
> For reference, folks can view the proposed libflashrom.h at
> https://review.coreboot.org/#/c/17946 to get a better idea for how these
> prefixes will look in libflashrom functions and data structures.
>
> Also, let's add "fi_" for flashrom interface to the list of proposed
> prefixes.
>
> My preferences (in order):
> 1. lf_
> 2. flashrom_
> 3. fi_
> 4. fl_
> 5. flash_
> 6. fr_
>
> IMO not only is "lf_" most intuitive, but the way the keys are spaced apart
> comfortably on qwerty, dvorak, and colemak layouts and each character
> (including the underscore) use a different hand to type. Same could be said
> about fl_ with regards to keyboard layout. fr_ is awkward (keys vertically
> adjacent) on qwerty and colemak, and fi_ is vertically adjacent on dvorak.
> flash_ and flashrom_ are not bad but are obviously many more keystrokes.
>
> The problem I have with fl_ is that it is also used for flash layout
> structs: https://review.coreboot.org/#/c/17944/4/layout.h . The layout
> structs are used internally and can be changed easily, though.
My bad. Only the `struct fl_layout` is part of the libflashrom inter-
face. I removed all other occurrences of the prefix for now from my
patches.
>
> flash_ is pretty good - For the most part it flows well with functions such
> as flash_image_read() and flash_image_write(), but is awkward with some
> other stuff like "flash_set_log_callback()". If we're already typing >2
> letters I think we ought to just use flashrom_ as the prefix to be
> complete, avoid awkward contexts, and avoid namespace conflicts (users
> might want to use flash_ in their code).
Agreed.
Nico
>
>
> On Sat, Apr 22, 2017 at 9:42 AM, Nico Huber <nico.h(a)gmx.de> wrote:
>
>> Hi flashrom folks,
>>
>> working again on implementing the libflashrom interface described here
>> [1]. During review [2] the question arose what `fl_` means and if we
>> don't want to use something else. The following alternatives were pro-
>> posed in the wiki:
>>
>> * fl_ / FL_ (probably *fl*ashrom)
>> * lf_ / LF_ (*l*ib *f*lashrom)
>> * lfr_ / LFR_ (*l*ib *f*lash *r*om)
>> * rom_ / ROM_
>>
>> IIRC, on IRC the following was proposed:
>>
>> * fr_ / FR_ (*f*lash *r*om)
>>
>> I don't think it has to be an acronym, so I'd add:
>>
>> * flash_ / FLASH_
>> * flashrom_ / FLASHROM_
>>
>> My personal preference would be either `fr_` or `flash_`.
>>
>> I think this will be open for discussion only for few days. Moving
>> forward is currently more important than finding the perfect name.
>>
>> Best regards,
>> Nico
>>
>> [1] https://www.flashrom.org/Libflashrom
>> [2] https://review.coreboot.org/#/c/17946/
>>
>> _______________________________________________
>> flashrom mailing list
>> flashrom(a)flashrom.org
>> https://mail.coreboot.org/mailman/listinfo/flashrom
>>
>
2 years, 1 month
Namespace prefix for libflashrom
by Nico Huber
Hi flashrom folks,
working again on implementing the libflashrom interface described here
[1]. During review [2] the question arose what `fl_` means and if we
don't want to use something else. The following alternatives were pro-
posed in the wiki:
* fl_ / FL_ (probably *fl*ashrom)
* lf_ / LF_ (*l*ib *f*lashrom)
* lfr_ / LFR_ (*l*ib *f*lash *r*om)
* rom_ / ROM_
IIRC, on IRC the following was proposed:
* fr_ / FR_ (*f*lash *r*om)
I don't think it has to be an acronym, so I'd add:
* flash_ / FLASH_
* flashrom_ / FLASHROM_
My personal preference would be either `fr_` or `flash_`.
I think this will be open for discussion only for few days. Moving
forward is currently more important than finding the perfect name.
Best regards,
Nico
[1] https://www.flashrom.org/Libflashrom
[2] https://review.coreboot.org/#/c/17946/
2 years, 1 month
Unknown error
by Андрей Макаровский
I had an error writing to flash chip sst49lf002a on chipset intel ich5: writing
sector at 0xb7410300. What could it be?
Best regards,
Andrew Makarovsky
2 years, 2 months
