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/
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.
On Sat, 15 Aug 2015 19:42:58 -0700
David Hendricks <david.hendricks(a)gmail.com> wrote:
> (I figured we could use a new thread to track overall progress)
>
> A few more tweaks and now writes work, too:
> https://chromium-review.googlesource.com/#/c/293889
FWIW, I have merged my refined version of Dediprog SF100 supporting the
new protocol. It also enables compilation of the Dediprog programmer by
default (finally).
--
Kind regards/Mit freundlichen Grüßen, Stefan Tauner
Hello ;)
I have a card called:
05:00.0 Ethernet controller [0200]: Intel Corporation I350 Gigabit Network Connection [8086:1521] (rev 01)
05:00.1 Ethernet controller [0200]: Intel Corporation I350 Gigabit Network Connection [8086:1521] (rev 01)
05:00.2 Ethernet controller [0200]: Intel Corporation I350 Gigabit Network Connection [8086:1521] (rev 01)
05:00.3 Ethernet controller [0200]: Intel Corporation I350 Gigabit Network Connection [8086:1521] (rev 01)
I update nicintel_spi.c to match product ID but when I try to probe it, flashroom freeze after a while:
./flashrom -VVV -p nicintel_spi:pci='05:00.0' -o probe.log
flashrom v0.9.8-r1916 on Linux 4.2.2-coreos-r1 (x86_64)
flashrom is free software, get the source code at https://flashrom.org
flashrom was built with libpci 3.2.1, GCC 4.8.4, little endian
Command line (5 args): ./flashrom -VVV -p nicintel_spi:pci=05:00.0 -o probe.log
Calibrating delay loop... OS timer resolution is 1 usecs, 1224M loops per second, 10 myus = 10 us, 100 myus = 102 us, 1000 myus = 1028 us, 10000 myus = 9996 us, 4 myus = 16 us, OK.
Initializing nicintel_spi programmer
Found "Intel I350 Gigabit Network Connection (rev 01)" (8086:1521, BDF 05:00.0).
===
This PCI device is UNTESTED. Please report the 'flashrom -p xxxx' output
to flashrom(a)flashrom.org if it works for you. Please add the name of your
PCI device to the subject. Thank you for your help!
===
PCI header type 0x00
Requested BAR is of type MEM, 32bit, not prefetchable
PCI header type 0x00
Requested BAR is of type MEM, 32bit, not prefetchable
page_size=1000
pre-rounding: start=0x00000000dfb70000, len=0x1000, end=0x00000000dfb71000
post-rounding: start=0x00000000dfb70000, len=0x1000, end=0x00000000dfb71000
Datasheet for this chipset can be found here:
http://www.intel.com/content/dam/www/public/us/en/documents/datasheets/ethe…
Also I found:
https://downloadcenter.intel.com/download/19186/Intel-Ethernet-Connections-…
When run it: ./bootutil64e
Intel(R) Ethernet Flash Firmware Utility
BootUtil version 1.5.72.1
Copyright (C) 2003-2015 Intel Corporation
Type BootUtil -? for help
Port Network Address Location Series WOL Flash Firmware Version
==== =============== ======== ======= === ============================= =======
1 002590E72B10 5:00.0 Gigabit YES FLASH Not Present
2 002590E72B11 5:00.1 Gigabit NO FLASH Not Present
3 002590E72B12 5:00.2 Gigabit NO FLASH Not Present
4 002590E72B13 5:00.3 Gigabit NO FLASH Not Present
Any chance to have support for this card by flashroom ?
Best Regards,
Grzegorz Hetman.
Atapromise chip size limiting has two bugs I didn't catch during review,
but only in the moment of commit.
The current code is checking model_id to remember if a chip has already
been limited, but if flashchips.c contains two subsequent chips with
different vendor_id but identical model_id the adjustment will not be
done. Switch to checking the chip size instead.
If a chip has multiple whole-chip erase functions, only one will be
modified. Fix that.
Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net>
Index: flashrom-atapromise_fix_limit_chip/atapromise.c
===================================================================
--- flashrom-atapromise_fix_limit_chip/atapromise.c (Revision 1916)
+++ flashrom-atapromise_fix_limit_chip/atapromise.c (Arbeitskopie)
@@ -79,38 +79,35 @@
static void atapromise_limit_chip(struct flashchip *chip)
{
- static uint32_t last_model_id = 0;
unsigned int i, size;
+ unsigned int usable_erasers = 0;
- if (chip->model_id == last_model_id)
- return;
-
size = chip->total_size * 1024;
- if (size > rom_size) {
- /* Undefine all block_erasers that don't operate on the whole chip,
- * and adjust the eraseblock size of the one that does.
- */
- for (i = 0; i < NUM_ERASEFUNCTIONS; ++i) {
- if (chip->block_erasers[i].eraseblocks[0].size != size) {
- chip->block_erasers[i].eraseblocks[0].count = 0;
- chip->block_erasers[i].block_erase = NULL;
- } else {
- chip->block_erasers[i].eraseblocks[0].size = rom_size;
- break;
- }
- }
- if (i != NUM_ERASEFUNCTIONS) {
- chip->total_size = rom_size / 1024;
- if (chip->page_size > rom_size)
- chip->page_size = rom_size;
+ /* Chip is small enough or already limited. */
+ if (size <= rom_size)
+ return;
+
+ /* Undefine all block_erasers that don't operate on the whole chip,
+ * and adjust the eraseblock size of those which do.
+ */
+ for (i = 0; i < NUM_ERASEFUNCTIONS; ++i) {
+ if (chip->block_erasers[i].eraseblocks[0].size != size) {
+ chip->block_erasers[i].eraseblocks[0].count = 0;
+ chip->block_erasers[i].block_erase = NULL;
} else {
- msg_pdbg("Failed to adjust size of chip \"%s\" (%d kB).\n", chip->name,
- chip->total_size);
+ chip->block_erasers[i].eraseblocks[0].size = rom_size;
+ usable_erasers++;
}
}
- last_model_id = chip->model_id;
+ if (usable_erasers) {
+ chip->total_size = rom_size / 1024;
+ if (chip->page_size > rom_size)
+ chip->page_size = rom_size;
+ } else {
+ msg_pdbg("Failed to adjust size of chip \"%s\" (%d kB).\n", chip->name, chip->total_size);
+ }
}
int atapromise_init(void)
On Wed, 27 Jan 2016 03:12:11 +0200
Urja Rannikko <urjaman(a)gmail.com> wrote:
> Hi,
>
> Review-ish... with some only-commentary included too.
>
> On Mon, Jan 18, 2016 at 1:28 AM, Stefan Tauner
> <stefan.tauner(a)alumni.tuwien.ac.at> wrote:
> > Signed-off-by: Urja Rannikko <urjaman(a)gmail.com>
> > Signed-off-by: Stefan Tauner <stefan.tauner(a)alumni.tuwien.ac.at>
> > ---
> > Makefile | 23 ++-
> > ch341a_spi.c | 531 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > flashrom.8.tmpl | 12 +-
> > flashrom.c | 12 ++
> > programmer.h | 13 ++
> > 5 files changed, 588 insertions(+), 3 deletions(-)
> > create mode 100644 ch341a_spi.c
> >
> > + free_idx = (free_idx + 1) % USB_IN_TRANSFERS; /* Increment (and wrap around). */
> Yeah this obvious construct is something I didnt use because I'm such
> an embedded guy that I'd never ever use % in performance related
> code... (i think i had some if idx>=USB_IN_TRANSFERS) idx = 0
> thing)... thanks for making it saner.
:)
> I had a fancier delay implementation done (all the way upto ms scale
> using the I2C commands), but since i didnt get that fully tested, this
> one will do, i'll post it as an improvement later.
Please do. I don't deem it that important but since you have
investigated the whole issue in such detail it would be a waste to not
exploit this knowledge.
> > + unsigned int i;
> > + for (i = 0; i < readcnt; i++) {
> > + /* FIXME: does working on words instead of bytes improve speed? */
> ... drop the comment? i dont really care, but i think our consensus on
> IRC was that a words-wide implementation would have more issues
> (alignment and such) than performance.
> Or make a comment with that info.
It's gone.
> > + *readarr++ = swap_byte(rbuf[writecnt + i]);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static const struct spi_master spi_master_ch341a_spi = {
> > + .type = SPI_CONTROLLER_CH341A_SPI,
> > + /* TODO: flashrom's current maximum is 256 B. Device was tested on Linux to accept atleast 16 kB. */
> > + .max_data_read = 16 * 1024,
> > + .max_data_write = 16 * 1024,
> On linux. Not on windows usb "stack" i think... set these back to 1k?
> Or test for the limit (maybe something like 4k?) and change the 1000ms
> timeout, since i think the big packet would timeout on windows if
> nothing else failed...
As discussed, I have tested up to 128 kB on Windows (in VM) and on
Linux with a bigger timeout and it works fine. We have agreed upon 4 kB
chunks now and using VLAs (I had an intermediate version that used
bigger chunks and relied on mallocs for them).
> With atleast the limits dropped (or tested), this is:
> Acked-by: Urja Rannikko <urjaman(a)gmail.com>
Thanks, r1921.
--
Kind regards/Mit freundlichen Grüßen, Stefan Tauner
On Mon, 25 Jan 2016 22:37:18 +0000
Simon Ramage <freeballsdeuce(a)yahoo.co.uk> wrote:
> Hello, hope you can help me, I have bricked my chromebox, been using a
> pomona clip via a raspberry pi on my spi, running flashrom, which finds my
> winbond chip but when i try to write to it, flashrom comes back with an
> error saying "image size (4096 B) doesn't match the flash chip's size
> (8388608 B)!"
That would mean that you try to write a 4 kB file to an 8 MB flash
chip. flashrom won't allow that for good reasons. It seems like you are
confusing two files IMHO. The 4 kB file is probably just the flash
descriptor but that alone is probably not what you want to write.
> and when i go to the properties of the image it is similar to
> the chip's size.
I certainly doubt that. It is more likely that you are comparing
something else than flashrom does (i.e. you are looking at the wrong
file).
> Have tried using hex editor on windows and flashing that
> format but keep getting same error message, hope you can help me with
> regard to this, thankyou.
You probably want to use a pre-built image by John Lewis. They should
have the correct size...
--
Kind regards/Mit freundlichen Grüßen, Stefan Tauner
