Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/25047
to look at the new patch set (#2).
Change subject: Support for Atmel / Adesto AT25SF041 SPI flash chip
......................................................................
Support for Atmel / Adesto AT25SF041 SPI flash chip
Change-Id: I0be930ff2258300508398e12fbe5abe10400fea2
Signed-off-by: jvm <git(a)jinvent.de>
---
M flashchips.c
M flashchips.h
2 files changed, 33 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/25047/2
--
To view, visit https://review.coreboot.org/25047
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0be930ff2258300508398e12fbe5abe10400fea2
Gerrit-Change-Number: 25047
Gerrit-PatchSet: 2
Gerrit-Owner: Julian von Mendel
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Nico Huber has posted comments on this change. ( https://review.coreboot.org/23865 )
Change subject: Fixed mingw detection on Windows 7 (NT-6.1) and hopefully in another non XP Windows build environments too.
......................................................................
Patch Set 1:
>> Patch Set 1:
>>
>> (7 comments)
> Thank you very much for the review Nico!
>
> Actually tgotic proposed a much simpler (and I think better)
> approach for checking the MingW environment:
> https://github.com/flashrom/flashrom/pull/33
>
> Is it possible to upload a new patchset if I am not an owner at the
> gerrit?
Yes, that works. Generally, you should ask the change's owner first,
but as you are the author anyway in this case, just do it.
--
To view, visit https://review.coreboot.org/23865
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f856dc4847c4ca9197b1935b7a9b9071b46c70a
Gerrit-Change-Number: 23865
Gerrit-PatchSet: 1
Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Márton Miklós <martonmiklosqdev(a)gmail.com>
Gerrit-Comment-Date: Wed, 07 Mar 2018 16:12:46 +0000
Gerrit-HasComments: No
Gerrit-HasLabels: No
Márton Miklós has posted comments on this change. ( https://review.coreboot.org/23865 )
Change subject: Fixed mingw detection on Windows 7 (NT-6.1) and hopefully in another non XP Windows build environments too.
......................................................................
Patch Set 1:
> Patch Set 1:
>
> (7 comments)
Thank you very much for the review Nico!
Actually tgotic proposed a much simpler (and I think better) approach for checking the MingW environment:
https://github.com/flashrom/flashrom/pull/33
Is it possible to upload a new patchset if I am not an owner at the gerrit?
--
To view, visit https://review.coreboot.org/23865
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f856dc4847c4ca9197b1935b7a9b9071b46c70a
Gerrit-Change-Number: 23865
Gerrit-PatchSet: 1
Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Márton Miklós <martonmiklosqdev(a)gmail.com>
Gerrit-Comment-Date: Wed, 07 Mar 2018 15:57:36 +0000
Gerrit-HasComments: No
Gerrit-HasLabels: No
Nico Huber has posted comments on this change. ( https://review.coreboot.org/23840 )
Change subject: Add "gingerly" flashing mode for the unreliable ISP environments
......................................................................
Patch Set 2:
Um, bigger things like this should actually be discussed on the mailing
list first, e.g. to avoid spending time on something that might not get
merged. Doesn't matter now, you already wrote it.
Regarding your original problem, I guess it could be solved with some
tuning of your hardware setup. Did you try to get the SoC into a better
state? (reset preferred; though, as you already work with two masters
on the SPI bus, did you try to boot the SoC up and halt the OS?).
About this implementation, when I read "gingerly" I expected something
more sophisticated (e.g. some sort of collision avoidance, listening on
the bus instead of jamming it). Though, I have nothing against the way
you do it, I would call it brute-force. Also, if I call `flashrom
--gingerly` I would expect something that is safer to use, but it kind
of is the opposite, isn't it?
I don't like to clutter up spi_read_chunked() and spi_write_chunked(),
most changes to them seem to be there only to avoid malloc() in your
spi_rw_gingerly()? I would rather move it there.
And... this might be the biggest issue: the possible endless loop in
spi_rw_gingerly(). For a mergeable solution, you'd have to put some
kind of timeout there. And that is where the problems start... if it
can fail, we have to handle the failure correctly, otherwise flashrom
(in its current implementation) would probably fall back to erase the
whole chip and make things worse.
Last but not least, why do it at the SPI level? Retries due to unre-
liable connections should be handled at a higher level, IMO.
--
To view, visit https://review.coreboot.org/23840
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3f18276d9fb7233d082720cb29d154f31c77100
Gerrit-Change-Number: 23840
Gerrit-PatchSet: 2
Gerrit-Owner: Mike Banon <mikebdp2(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Mike Banon <mikebdp2(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Wed, 07 Mar 2018 14:01:41 +0000
Gerrit-HasComments: No
Gerrit-HasLabels: No